Re: [PATCH v6 08/22] drm: bridge: dw_hdmi: Unregister CEC notifier during connector cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving `cec_notifier_conn_unregister` to the connector's `.destroy` callback introduces an asynchronous state corruption vulnerability, breaking CEC functionality on driver rebind.
--

commit 8c5f1d529adb3a91696d1e624a0b3dc31d5287ba
Author: Jonas Karlman <jonas@kwiboo.se>
drm: bridge: dw_hdmi: Unregister CEC notifier during connector cleanup

This commit moves the cec_notifier_conn_unregister call from the bridge
detach function to the connector destroy function. This is intended to align
the lifetime of the connector and the CEC notifier.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index cbbd155780425..5fd26ff8f55b4 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2532,6 +2532,11 @@ static void dw_hdmi_connector_destroy(struct drm_connector *connector)
>  {
>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
>  
> +	mutex_lock(&hdmi->cec_notifier_mutex);
> +	cec_notifier_conn_unregister(hdmi->cec_notifier);

Could this cause state corruption if a user holds the DRM file descriptor
open across a driver unbind/bind cycle?

Because the connector destroy callback runs asynchronously when the last DRM
file descriptor is closed, its execution can be delayed indefinitely.

If the driver is unbound and then rebound while the old file descriptor is
still open, a new dw_hdmi is allocated and a new connector is created. When
cec_notifier_conn_register is called on rebind, it looks up the notifier by
the physical device (hdmi->dev) and shares the exact same cec_notifier with
the new active pipeline.

When the user eventually closes the old file descriptor, the old connector's
destroy callback will fire and call cec_notifier_conn_unregister. This will
clear the connection info and invalidate the physical address, which would
destroy the active CEC state of the newly bound display pipeline.

Should hardware-coupled state like the CEC notifier be invalidated at
hardware boundaries like detach, rather than software refcount boundaries
like destroy?

> +	hdmi->cec_notifier = NULL;
> +	mutex_unlock(&hdmi->cec_notifier_mutex);
> +
>  	drm_connector_cleanup(connector);
>  	drm_bridge_put(&hdmi->bridge);
>  }

[ ... ]

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic