Re: [PATCH v8 02/21] drm/bridge: synopsys: dw-dp: Fix incorrect resource lifetimes in bind callback

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

New issues:
- [High] Moving IRQ registration and hardware initialization to probe creates a window where an HPD interrupt causes a NULL pointer dereference on `dp->bridge.dev`.

Pre-existing issues:
- [Critical] Use-After-Free of the `dp` structure due to uncancelled `hpd_work` on driver removal.
--

--- Patch [2]: [PATCH v8 02/21] drm/bridge: synopsys: dw-dp: Fix incorrect resource lifetimes in bind callback ---
commit 9de3c09754cbe88e934e9c5f4adf581795df6d41
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 the whole
bind function.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 112a499113095..7f82495141711 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c

[ ... ]

> @@ -2091,11 +2094,19 @@ struct dw_dp *dw_dp_bind(struct device *dev, struct drm_encoder *encoder,
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> -	dp->irq = platform_get_irq(pdev, 0);
> -	if (dp->irq < 0) {
> -		ret = dp->irq;
> +	bridge = &dp->bridge;
> +	bridge->of_node = dev->of_node;
> +	bridge->ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
> +	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
> +	bridge->ycbcr_420_allowed = true;
> +
> +	ret = devm_drm_bridge_add(dev, bridge);
> +	if (ret)
>  		return ERR_PTR(ret);
> -	}
> +
> +	dp->irq = platform_get_irq(pdev, 0);
> +	if (dp->irq < 0)
> +		return ERR_PTR(dp->irq);
>  
>  	ret = devm_request_threaded_irq(dev, dp->irq, NULL, dw_dp_irq,
>  					IRQF_ONESHOT, dev_name(dev), dp);

[Severity: High]
Can this IRQ registration create a window for a NULL pointer dereference?

If a monitor is connected, the HPD interrupt can fire immediately after
devm_request_threaded_irq() completes in dw_dp_probe(). The interrupt handler
schedules dw_dp_hpd_work(), which directly dereferences dp->bridge.dev:

dw_dp_hpd_work()
   drm_helper_hpd_irq_event(dp->bridge.dev);

Since the component bind hasn't run yet, drm_bridge_attach() hasn't been
called to assign dp->bridge.dev. Could this cause dw_dp_hpd_work() to
dereference a NULL pointer if an interrupt fires before the bridge is
attached?

> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index b23efb153c9e6..005938dc66c9f 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c

[ ... ]

> @@ -128,12 +119,30 @@ static const struct component_ops dw_dp_rockchip_component_ops = {
>  	.unbind = dw_dp_rockchip_unbind,
>  };
>  
> -static int dw_dp_probe(struct platform_device *pdev)
> +static int dw_dp_rockchip_probe(struct platform_device *pdev)
>  {

[ ... ]

>  }
>  
> -static void dw_dp_remove(struct platform_device *pdev)
> +static void dw_dp_rockchip_remove(struct platform_device *pdev)
>  {
>  	component_del(&pdev->dev, &dw_dp_rockchip_component_ops);
>  }

[Severity: Critical]
This is a pre-existing issue, but does this remove function leave a pending
work item that can cause a use-after-free?

Since there is no explicit cancel_work_sync(&dp->hpd_work) before the driver
exits dw_dp_rockchip_remove(), if an HPD interrupt fires right before removal,
the scheduled dw_dp_hpd_work() might remain on the system workqueue.

The devm cleanup runs after this function returns, freeing the IRQ and the dp
structure. If the work executes after dp is freed, it will access freed memory
when it runs container_of(work, struct dw_dp, hpd_work).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-synopsys-dw-dp-improvements-v8-0-ac1e6a75782f@collabora.com?part=2