Re: [PATCH v2 36/37] drm/sti: hdmi: Switch to atomic bridge callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak and global list corruption (dangling pointer) if component_add() fails in sti_hdmi_probe().
- [High] Resource leak of the audio platform device and potential NULL pointer dereference if sti_hdmi_bind() fails.
--

--- Patch [36]: [PATCH v2 36/37] drm/sti: hdmi: Switch to atomic bridge callbacks ---
commit fe4cd2364e238c4d447b17cda3c258ebb7c65628
Author: Maxime Ripard <mripard@kernel.org>

drm/sti: hdmi: Switch to atomic bridge callbacks

The hdmi bridge uses the legacy non-atomic pre_enable, enable, disable
and post_disable bridge callbacks.

In order to remove the legacy bridge callback support from the DRM
bridge core, switch to their atomic counterparts and add the bridge
atomic state handlers.

> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index f8222e60b1e01..7ba9e0b7ba6c0 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c

[ ... ]

> @@ -964,16 +966,20 @@ static void sti_hdmi_set_mode(struct drm_bridge *bridge,
>  	}
>  }
>  
> -static void sti_hdmi_bridge_nope(struct drm_bridge *bridge)
> +static void sti_hdmi_bridge_nope(struct drm_bridge *bridge,
> +				 struct drm_atomic_commit *commit)
>  {
>  	/* do nothing */
>  }
>  
>  static const struct drm_bridge_funcs sti_hdmi_bridge_funcs = {
> -	.pre_enable = sti_hdmi_pre_enable,
> -	.enable = sti_hdmi_bridge_nope,
> -	.disable = sti_hdmi_disable,
> -	.post_disable = sti_hdmi_bridge_nope,
> +	.atomic_create_state = drm_atomic_helper_bridge_create_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_pre_enable = sti_hdmi_pre_enable,
> +	.atomic_enable = sti_hdmi_bridge_nope,
> +	.atomic_disable = sti_hdmi_disable,
> +	.atomic_post_disable = sti_hdmi_bridge_nope,
>  	.mode_set = sti_hdmi_set_mode,
>  };

[Severity: High]
This is a pre-existing issue, but does sti_hdmi_probe() leak memory and
leave a dangling pointer if component_add() fails?

drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_probe() {
	...
	drm_bridge_add(&hdmi->bridge);
	return component_add(&pdev->dev, &sti_hdmi_ops);
}

If component_add() fails, the function returns an error without calling
drm_bridge_remove().

Because the bridge is never removed, the reference count does not drop to
zero during devres cleanup. Could another driver try to access the corrupted
bridge in the global bridge_list, leading to a use-after-free panic when it
accesses the freed devres-managed resources?

[Severity: High]
This isn't a bug introduced by this patch, but does the error handling in
sti_hdmi_bind() leave the audio platform device registered while setting the
connector to NULL?

drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_bind() {
	...
err_sysfs:
	hdmi->drm_connector = NULL;
	return -EINVAL;
}

If sti_hdmi_register_audio_driver() succeeds but a subsequent initialization
step fails, we jump to the err_sysfs label, which clears hdmi->drm_connector
without unregistering the active audio device.

Since the ALSA framework may still invoke operations on the registered codec,
can this trigger a NULL pointer dereference when hdmi_audio_get_eld() is
called?

drivers/gpu/drm/sti/sti_hdmi.c:hdmi_audio_get_eld() {
	...
	struct drm_connector *connector = hdmi->drm_connector;

	DRM_DEBUG_DRIVER("\n");
	mutex_lock(&connector->eld_mutex);
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=36