Re: [PATCH v6 13/22] drm: bridge: dw_hdmi: Use generic CEC notifier helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-After-Free and Memory Leak in CEC notifier cleanup due to drmm and manual lifecycle mismatch
--

commit eb59f6fe976b03aa5f719b79a28355b27f403bfe
Author: Jonas Karlman <jonas@kwiboo.se>
Subject: drm: bridge: dw_hdmi: Use generic CEC notifier helpers

This commit replaces open-coded CEC notifier handling with generic CEC
helpers using drmm_connector_hdmi_cec_notifier_register. It also removes
the manual notifier unregistration from dw_hdmi_connector_destroy.

> The drmm release action for the generic CEC notifier should run just
> before dw_hdmi_connector_destroy(), closely matching the lifetime of
> the replaced CEC notifier and the connector.

Is this lifecycle assumption accurate for drivers that do not use drmm
for mode config?

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
[ ... ]
> @@ -2547,9 +2543,6 @@ static void dw_hdmi_connector_destroy(struct drm_connector *connector)
>  {
>  	struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector);
>  
> -	cec_notifier_conn_unregister(hdmi->cec_notifier);
> -	hdmi->cec_notifier = NULL;
> -
>  	drm_connector_cleanup(connector);
>  	drm_bridge_put(&hdmi->bridge);
>  }

Since drm_connector_cleanup() zeroes the connector memory, and
drm_bridge_put() can drop the final reference to the bridge (which frees
the dw_hdmi structure and its embedded connector), could this lead to a
use-after-free?

If a user-space application holds an open file descriptor to the DRM device
during driver unbind, the DRM device remains alive. The deferred drmm
release action will only execute later when the file descriptor is closed
and the DRM device's refcount drops to zero.

When the drmm action eventually runs, it will dereference the freed
connector pointer.

Additionally, because drm_connector_cleanup() executes
memset(connector, 0, sizeof(*connector)), won't connector->cec.data become
NULL? This would cause the delayed drmm release action to silently skip
unregistration, leaking the cec_notifier object.

[ ... ]
> @@ -2609,15 +2600,8 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
[ ... ]
> -	notifier = cec_notifier_conn_register(hdmi->dev, NULL, &conn_info);
> -	if (!notifier)
> -		return -ENOMEM;
> -
> -	hdmi->cec_notifier = notifier;
> -
> -	return 0;
> +	return drmm_connector_hdmi_cec_notifier_register(connector, NULL,
> +							 hdmi->dev);
>  }

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