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

The commit 8b1a8f8b2002 ("drm/display: add CEC helpers code") added
generic CEC helpers to be used by HDMI drivers.

Replace the open-coded CEC notifier handling with use of the generic CEC
notifier helpers. Ensure DRM_DISPLAY_HDMI_CEC_NOTIFIER_HELPER is also
selected when DRM_DW_HDMI_CEC is enabled so that the CEC helpers is
available.

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.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Tested-by: Diederik de Haas <diederik@cknow-tech.com>  # Rock64, RockPro64, Quartz64-B
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v6: Update commit message,
    Collect t-b tag
v5: Collect r-b tag
v4: New patch
---
 drivers/gpu/drm/bridge/synopsys/Kconfig   |  1 +
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 26 +++++------------------
 2 files changed, 6 insertions(+), 21 deletions(-)
  

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);
>  }
  

On 5/16/2026 9:20 PM, sashiko-bot@kernel.org wrote:
> 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?

Yes, drm_mode_config_init() was converted to call its drmm_ variant in
v5.8-rc1, with commit c3b790ea07a1 ("drm: Manage drm_mode_config_init
with drmm_"). Effectively meaning that all drm drivers 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?

It should not, destroy() is the last call made for the connector, and
the drm_bridge_put() is here to ensure that the struct dw_hdmi is still
around when drm_connector_cleanup() is called, to avoid 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.

It should not, drmm release actions are called in reverse order, and
drmm_connector_hdmi_cec_notifier_register() is called last, meaning its
release action should be called before drm_mode_config_cleanup() is
called by the the release action for drmm_mode_config_init().

Regards,
Jonas

> [ ... ]
>> @@ -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);
>>  }
>