Re: [PATCH v2 27/37] drm/mcde: dsi: Add atomic bridge state handlers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `drm_bridge_remove()` in driver unbind leads to memory leak and Use-After-Free via dangling global list pointer.
--

--- Patch [27]: [PATCH v2 27/37] drm/mcde: dsi: Add atomic bridge state handlers ---
commit 6ab8c58e40b6724f223de55f6cba1eb9cd4b73eb
Author: Maxime Ripard <mripard@kernel.org>

    drm/mcde: dsi: Add atomic bridge state handlers

This commit adds atomic bridge state handlers (atomic_create_state,
atomic_destroy_state, atomic_duplicate_state) to the mcde_dsi bridge. This
helps facilitate the removal of legacy bridge callback support from the DRM
bridge core.

> diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
> index 47d45897ed069..5cf44ccb02cf9 100644
> --- a/drivers/gpu/drm/mcde/mcde_dsi.c
> +++ b/drivers/gpu/drm/mcde/mcde_dsi.c
> @@ -1063,6 +1063,9 @@ static int mcde_dsi_bridge_attach(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs mcde_dsi_bridge_funcs = {
> +	.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,
>  	.attach = mcde_dsi_bridge_attach,
>  	.mode_set = mcde_dsi_bridge_mode_set,
>  };

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the bridge
operations I noticed a pre-existing issue with how the bridge is removed.

In mcde_dsi_bind(), the bridge is registered and added to the global bridge
list:

drivers/gpu/drm/mcde/mcde_dsi.c:mcde_dsi_bind() {
	...
	/* Create a bridge for this DSI channel */
	d->bridge.of_node = dev->of_node;
	drm_bridge_add(&d->bridge);
	...
}

However, in mcde_dsi_unbind(), there is no corresponding call to
drm_bridge_remove():

drivers/gpu/drm/mcde/mcde_dsi.c:mcde_dsi_unbind() {
	struct mcde_dsi *d = dev_get_drvdata(dev);

	if (d->panel)
		drm_panel_bridge_remove(d->bridge.next_bridge);
	regmap_update_bits(d->prcmu, PRCM_DSI_SW_RESET,
			   PRCM_DSI_SW_RESET_DSI0_SW_RESETN, 0);
}

Does this leave the bridge in the global bridge_list, leading to a memory
leak when unbinding the driver?

Furthermore, if another driver traverses the global DRM bridge list after this
driver is unloaded, could it dereference the dangling pointers (like
bridge->dev and bridge->funcs) and cause a Use-After-Free kernel panic?

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