Re: [PATCH v2 1/5] drm/panel: have drm_panel_add/remove manage a list reference

[sashiko-bot@kernel.org]

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

New issues:
- [High] Acquiring a reference in `drm_panel_add()` creates 'zombie' panels on driver probe failure, leading to deferred Use-After-Free of devres-managed resources.

Pre-existing issues:
- [High] `of_drm_find_panel()` and `find_panel_by_fwnode()` return un-refcounted pointers, leaving consumers vulnerable to TOCTOU Use-After-Free.
--

--- Patch [1]: [PATCH v2 1/5] drm/panel: have drm_panel_add/remove manage a list reference ---
commit b7da36095272a2101eb01a96e0511080d18be813
Author: Albert Esteve <aesteve@redhat.com>
Subject: drm/panel: have drm_panel_add/remove manage a list reference

This commit updates the panel registry to manage a reference count for
struct drm_panel objects while they are present in the global list. By
calling drm_panel_get() on addition and drm_panel_put() on removal, it
attempts to prevent panels from being freed while still linked.

> The global registry now holds a counted reference for as long as the
> panel is listed, ensuring the object outlives any concurrent lookup.

[Severity: High]
This is a pre-existing issue, but does the global list reference actually
ensure the object outlives the consumer's use of it?

Looking at of_drm_find_panel() and find_panel_by_fwnode(), they search the
list under panel_lock and return the pointer without acquiring a reference
for the caller:

of_drm_find_panel() {
	mutex_lock(&panel_lock);
	...
			mutex_unlock(&panel_lock);
			return panel;
}

As soon as panel_lock is unlocked, a concurrent driver unbind could call
drm_panel_remove(), which deletes the panel from the list and drops its
refcount. If the panel is then freed, won't the consumer thread that called
of_drm_find_panel() be left holding a dangling pointer?

> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 2c5649e433dfb..545fe93dc28fe 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -81,6 +81,7 @@ static void drm_panel_init(struct drm_panel *panel, struct device *dev,
>   */
>  void drm_panel_add(struct drm_panel *panel)
>  {
> +	drm_panel_get(panel);

[Severity: High]
Does acquiring a reference here create zombie panels if a driver's probe
fails after adding the panel?

For example, if a panel driver successfully calls drm_panel_add() but fails a
subsequent probe step (like mipi_dsi_attach) and forgets to call
drm_panel_remove(), the device's probe fails. The devres framework will then
free all devm-managed hardware resources like regulators, clocks, and GPIOs.

However, because the list now holds a reference, the struct drm_panel memory
is kept alive and remains in the registry. If a display controller later
looks up this zombie panel via of_drm_find_panel() and invokes its prepare()
or enable() callbacks, will it dereference those freed devres pointers?

>  	mutex_lock(&panel_lock);
>  	list_add_tail(&panel->list, &panel_list);
>  	mutex_unlock(&panel_lock);

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