Re: [PATCH v2 2/5] drm/bridge/panel: hold a reference to the wrapped panel

[sashiko-bot@kernel.org]

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

New issues:
- [High] Dereference of `panel_bridge` after it may have been freed inside `devm_drm_panel_bridge_release()`.

Pre-existing issues:
- [High] `drm_panel_bridge_remove()` accesses `panel->dev` for devres removal, but the underlying `struct device` may have already been freed.
--

--- Patch [2]: [PATCH v2 2/5] drm/bridge/panel: hold a reference to the wrapped panel ---
commit 94f4ef45af3a89d595cd95811619b47c422771c3
Author: Albert Esteve <aesteve@redhat.com>

drm/bridge/panel: hold a reference to the wrapped panel

This patch acquires a reference to the wrapped drm_panel in
drm_panel_bridge_add_typed() using drm_panel_get() and releases it in
the teardown paths to prevent dereferencing dangling pointers.

> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index 4978ec98a0828..6b98ad19508df 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
[ ... ]
> @@ -316,6 +316,7 @@ EXPORT_SYMBOL(drm_panel_bridge_add_typed);
>  void drm_panel_bridge_remove(struct drm_bridge *bridge)
>  {
>  	struct panel_bridge *panel_bridge;
> +	struct drm_panel *panel;
>  
>  	if (!bridge)
>  		return;
> @@ -326,10 +327,12 @@ void drm_panel_bridge_remove(struct drm_bridge *bridge)
>  	}
>  
>  	panel_bridge = drm_bridge_to_panel_bridge(bridge);
> +	panel = panel_bridge->panel;
>  
>  	drm_bridge_remove(bridge);
>  	/* TODO remove this after reworking panel_bridge lifetime */
> -	devm_drm_put_bridge(panel_bridge->panel->dev, bridge);
> +	devm_drm_put_bridge(panel->dev, bridge);

[Severity: High]
This is a pre-existing issue, but can accessing panel->dev here lead to
a use-after-free of the struct device?

While this patch successfully keeps the drm_panel structure alive via
drm_panel_get(), it does not appear to take a get_device() reference to
the underlying struct device (panel->dev). 

If the panel device was completely unregistered, could the struct device
memory be freed while the drm_panel object remains alive, causing
devm_drm_put_bridge() to operate on a dangling pointer?

> +	drm_panel_put(panel);
>  }
>  EXPORT_SYMBOL(drm_panel_bridge_remove);
>  
> @@ -357,11 +360,14 @@ EXPORT_SYMBOL(drm_panel_bridge_set_orientation);
>  static void devm_drm_panel_bridge_release(struct device *dev, void *res)
>  {
>  	struct drm_bridge *bridge = *(struct drm_bridge **)res;
> +	struct panel_bridge *panel_bridge;
>  
>  	if (!bridge)
>  		return;
>  
> +	panel_bridge = drm_bridge_to_panel_bridge(bridge);
>  	drm_bridge_remove(bridge);
> +	drm_panel_put(panel_bridge->panel);

[Severity: High]
Could this result in a use-after-free of the panel_bridge structure?

If the consumer device unbinds, devm_drm_panel_bridge_release() calls
drm_bridge_remove(bridge). If this drops the final reference to the
bridge, it triggers __drm_bridge_free() which frees the panel_bridge
structure.

The very next line calls drm_panel_put(panel_bridge->panel). Does this
dereference the potentially freed panel_bridge pointer?

Perhaps panel_bridge->panel should be cached in a local variable before
calling drm_bridge_remove()?

>  }
>  
>  /**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-drm_refcount_wiring-v2-0-d3bb61f4bd4d@redhat.com?part=2