Re: [PATCH v8 03/21] drm/bridge: synopsys: dw-dp: Cancel pending HPD work

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The HPD worker `dw_dp_hpd_work` unconditionally accesses `dp->bridge.dev`, causing a NULL pointer dereference or use-after-free.
- [High] Missing `reinit_completion()` in `dw_dp_aux_transfer()` causes premature completions after a previous timeout.
- [High] Sleeping functions are called from within the `dw_dp_bridge_atomic_disable` callback.
--

--- Patch [3]: [PATCH v8 03/21] drm/bridge: synopsys: dw-dp: Cancel pending HPD work ---
commit 2a199fd0ed454a3e93c9003823239c66f8489c2a
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

drm/bridge: synopsys: dw-dp: Cancel pending HPD work

There is a race condition during device removal: If the HPD interrupt
started an HPD worker exactly when the device is removed, the worker
thread might access invalid resources. Avoid this by stopping any
pending work immediately after disabling the interrupt.

Fixes: 86eecc3a9c2e ("drm/bridge: synopsys: Add DW DPTX Controller support library")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 7f82495141711..6289ed06e2317 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> @@ -2027,7 +2028,6 @@ struct dw_dp *dw_dp_probe(struct platform_device *pdev, const struct dw_dp_plat_
>  	dp->plat_data.max_link_rate = plat_data->max_link_rate;
>  
>  	mutex_init(&dp->irq_lock);
> -	INIT_WORK(&dp->hpd_work, dw_dp_hpd_work);
>  	init_completion(&dp->complete);

[Severity: High]
This is a pre-existing issue, but can dw_dp_aux_transfer() complete 
prematurely after a timeout?

We initialize the completion here, but dw_dp_aux_transfer() calls 
wait_for_completion_timeout(&dp->complete, timeout) without calling 
reinit_completion() before starting the transfer.

If a previous transfer times out, but a delayed hardware interrupt 
later calls complete(&dp->complete), won't the next call to 
wait_for_completion_timeout() return immediately with the stale 
completion state?

>  
>  	res = devm_platform_ioremap_resource(pdev, 0);
[ ... ]
> @@ -2108,6 +2108,10 @@ struct dw_dp *dw_dp_probe(struct platform_device *pdev, const struct dw_dp_plat_
>  	if (dp->irq < 0)
>  		return ERR_PTR(dp->irq);
>  
> +	ret = devm_work_autocancel(dev, &dp->hpd_work, dw_dp_hpd_work);

[Severity: High]
This isn't a bug introduced by this patch, but does the devres cleanup 
still leave a window for a NULL pointer dereference in the worker?

Looking at dw_dp_hpd_work(), it unconditionally accesses dp->bridge.dev:

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

And in dw_dp_link_retrain(), it does:

dw_dp_link_retrain()
   ret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);

During component bind/unbind, drm_bridge_attach() sets up and tears down 
bridge->dev. Since component_del() is typically called before devres 
actions, the DRM device is unbound and bridge->dev becomes NULL before 
this devm_work_autocancel() takes effect. Does this allow a pending HPD 
interrupt to execute the worker against a NULL or freed bridge->dev 
during teardown?

> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	ret = devm_request_threaded_irq(dev, dp->irq, NULL, dw_dp_irq,
>  					IRQF_ONESHOT, dev_name(dev), dp);
>  	if (ret) {

[Severity: High]
This is a pre-existing issue, but can the atomic disable path sleep?

According to the DRM subsystem rules, atomic disable callbacks run in 
atomic context and cannot sleep. However, the driver's 
dw_dp_bridge_atomic_disable() unconditionally calls dw_dp_reset(), which 
contains sleeping functions:

dw_dp_bridge_atomic_disable()
   dw_dp_reset()
      usleep_range(10, 20);

Since usleep_range() and disable_irq() can sleep, does this cause kernel 
warnings or deadlocks when the DRM core disables the display pipeline 
during modesetting?

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