Re: [PATCH v7 15/23] 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 due to drmm action lifetime mismatch with the connector
--

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

This commit replaces the open-coded CEC notifier handling with the generic
DRM HDMI CEC notifier helpers.

> 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 assumption accurate for all drivers using dw_hdmi?

When master drivers like sun4i or imx are unbound, drm_mode_config_cleanup()
is often called synchronously, which in turn calls dw_hdmi_connector_destroy().

If a userspace process holds the DRM file descriptor open, the drm_device
outlives the unbind. Doesn't this mean the drmm release action will run
much later than dw_hdmi_connector_destroy(), rather than just before it?

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 0e84dff724706..37406555af7bc 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
[ ... ]
> @@ -2587,15 +2578,8 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi)
>  
>  	drm_connector_attach_encoder(connector, hdmi->bridge.encoder);
>  
> -	cec_fill_conn_info_from_drm(&conn_info, connector);
> -
> -	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);
>  }

Could this cause a use-after-free when the drm_device is finally released?

Looking at dw_hdmi_connector_destroy(), it cleans up the connector state:

dw_hdmi_connector_destroy()
  drm_connector_cleanup(connector)
    ...
    memset(connector, 0, sizeof(*connector));

Since the connector is embedded in struct dw_hdmi which is managed by devres
and the bridge refcount, the memory will be freed when the platform device
unbind completes.

When the drmm action drm_connector_hdmi_cec_notifier_unregister() eventually
executes during drm_dev_release(), won't it attempt to dereference the
already freed connector pointer?

If the memory happens to still be accessible, won't it read the zeroed
connector->cec.data as a NULL pointer, silently leaking the CEC notifier?

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

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