Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Missing cancel_work_sync(&dp->hpd_work) in dw_dp_unbind leads to 
use-after-free
- [High] Missing reinit_completion() in AUX transfer function causes stale 
completion state
--

--- Patch [1]: [PATCH v5 01/15] drm/bridge: synopsys: dw-dp: Fix incorrect 
resource lifetimes in bind callback ---
commit f51b82393e0acdef468182d74c204f63a5e111ae
Author: Sebastian Reichel <[email protected]>

drm/bridge: synopsys: dw-dp: Fix incorrect resource lifetimes in bind callback

Currently the Synopsys DesignWare DP controller driver's bind function
requests lots of resources using device managed functions. These are
free'd on driver removal instead of at unbind time. Fix this discrepancy
by introducing a new probe helper function and moving over most of the
early resource acquisitions.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 3445c82e6f50e..368e1f85e6253 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c

[ ... ]

> +     dp->aux.transfer = dw_dp_aux_transfer;

[Severity: High]
This isn't a bug introduced by this patch, but does the AUX transfer function
need a call to reinit_completion()?

Looking at dw_dp_aux_transfer() in drivers/gpu/drm/bridge/synopsys/dw-dp.c,
it waits for completion like this:

        regmap_write(dp->regmap, DW_DP_AUX_CMD, value);
        status = wait_for_completion_timeout(&dp->complete, timeout);

If a previous transfer times out and the hardware interrupt eventually arrives
late, it can leave dp->complete.done greater than zero. When the next transfer
is initiated, wait_for_completion_timeout() could consume the leftover
completion and return immediately without waiting for the new transfer to
finish, causing it to read garbage DPCD data.

> +     ret = drm_dp_aux_register(&dp->aux);

[ ... ]

> +void dw_dp_unbind(struct dw_dp *dp)
> +{
> +     disable_irq(dp->irq);
>       phy_exit(dp->phy);
> +     drm_dp_aux_unregister(&dp->aux);
> +     drm_bridge_remove(&dp->bridge);
>  }

[Severity: High]
This is a pre-existing issue, but does dw_dp_unbind() in
drivers/gpu/drm/bridge/synopsys/dw-dp.c need to call
cancel_work_sync(&dp->hpd_work) to prevent a use-after-free?

If a hotplug event occurs when the driver is being unbound, the IRQ handler
might schedule dp->hpd_work. The disable_irq() call here waits for the IRQ
handler to finish but does not cancel pending workqueue items. After the
driver unbinds and the devm-managed dp structure is freed, the workqueue
could execute dw_dp_hpd_work() and access the freed dp structure.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/20260724-synopsys-dw-dp-improvements-v5-0-9445c2e87...@collabora.com?part=1

Reply via email to