Re: [PATCH v5 16/21] drm: bridge: dw_hdmi: Use delayed_work to debounce hotplug event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Asynchronous disable_delayed_work() in hpd_disable callbacks allows the hotplug work to run concurrently with DRM teardown, causing a Use-After-Free.
--

commit 3caa1bb48d6eede4b5fef2227a86e8e2d0b2a913
Author: Jonas Karlman <jonas@kwiboo.se>

drm: bridge: dw_hdmi: Use delayed_work to debounce hotplug event

This commit introduces a delayed work item to debounce HDMI hotplug events.
It enables the work during the enable_hpd phase and disables it asynchronously
during the disable_hpd phase, and finally cancels it synchronously when the
module is removed or suspended.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index a6f2702bca83c..f32eea9106b00 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
[ ... ]
> @@ -2517,6 +2520,20 @@ static void dw_hdmi_connector_force(struct drm_connector *connector)
>  	dw_hdmi_connector_status_update(connector, connector->status);
>  }
>  
> +static void dw_hdmi_connector_enable_hpd(struct drm_connector *connector)
> +{
> +	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
> +
> +	enable_delayed_work(&hdmi->hpd_work);
> +}
> +
> +static void dw_hdmi_connector_disable_hpd(struct drm_connector *connector)
> +{
> +	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
> +
> +	disable_delayed_work(&hdmi->hpd_work);

Could this cause a use-after-free during teardown?

Since disable_delayed_work() cancels the work asynchronously without waiting
for it to finish, if the work is currently executing on another cpu, it could
access the drm device after it has been freed during unbind.

> +}
[ ... ]
> @@ -2959,6 +2978,20 @@ static const struct drm_edid *dw_hdmi_bridge_edid_read(struct drm_bridge *bridge
>  	return dw_hdmi_edid_read(hdmi, connector);
>  }
>  
> +static void dw_hdmi_bridge_hpd_enable(struct drm_bridge *bridge)
> +{
> +	struct dw_hdmi *hdmi = bridge->driver_private;
> +
> +	enable_delayed_work(&hdmi->hpd_work);
> +}
> +
> +static void dw_hdmi_bridge_hpd_disable(struct drm_bridge *bridge)
> +{
> +	struct dw_hdmi *hdmi = bridge->driver_private;
> +
> +	disable_delayed_work(&hdmi->hpd_work);

Similarly, would this asynchronous cancel allow the hpd_work to run
concurrently with drm teardown?

Because the synchronous cancel_delayed_work_sync() in dw_hdmi_remove() is
executed only after the master unbind fully completes, the work could still
be executing on another cpu.

When the work accesses hdmi->bridge.dev:

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c:dw_hdmi_hpd_work() {
    ...
	drm_helper_hpd_irq_event(hdmi->bridge.dev);
    ...
}

Could it dereference an already freed drm device?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510124111.1226584-1-jonas@kwiboo.se?part=16