Re: [PATCH v2 4/5] drm/panel: find_panel_by_fwnode() return a counted reference

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Drivers calling of_drm_find_panel() directly leak the returned panel reference.

Pre-existing issues:
- [High] TOCTOU race in drm_panel_add_follower() when acquiring the panel's underlying device, leading to a potential Use-After-Free of panel->dev.
--

--- Patch [4]: [PATCH v2 4/5] drm/panel: find_panel_by_fwnode() return a counted reference ---
commit 5cd4c479944cf29f6f5aa4a3e31207bf48dc0567
Author: Albert Esteve <aesteve@redhat.com>

drm/panel: find_panel_by_fwnode() return a counted reference

This commit updates find_panel_by_fwnode() to return a refcounted pointer
to the panel, mirroring a similar fix made to of_drm_find_panel(). It calls
drm_panel_get() under the panel_lock before returning, ensuring the memory
is not freed prematurely. Callers are now responsible for balancing this
with drm_panel_put(), which is added to drm_panel_remove_follower().

> diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
> index 72cf86253c0cd..a00ae98ed0956 100644
> --- a/drivers/gpu/drm/drm_panel.c
> +++ b/drivers/gpu/drm/drm_panel.c
> @@ -542,7 +542,13 @@ int of_drm_get_panel_orientation(const struct device_node *np,
>  EXPORT_SYMBOL(of_drm_get_panel_orientation);
>  #endif
>  
> -/* Find panel by fwnode. This should be identical to of_drm_find_panel(). */
> +/*
> + * Find panel by fwnode, returning a counted reference.
> + *
> + * Behaves identically to of_drm_find_panel(). On success the returned
> + * pointer has been passed through drm_panel_get(); the caller must call
> + * drm_panel_put() when done with it.
> + */

[Severity: Medium]
Since the earlier patch in this series updated of_drm_find_panel() to
also return a counted reference, do drivers that call it directly need
to drop their reference to prevent a memory leak?

For example, in bridge drivers like lvds-codec.c:

drivers/gpu/drm/bridge/lvds-codec.c:lvds_codec_probe() {
    panel = of_drm_find_panel(panel_node);
    of_node_put(panel_node);
    ...
    lvds_codec->panel_bridge = devm_drm_panel_bridge_add_typed(dev, panel,
                                                lvds_codec->connector_type);
}

Because devm_drm_panel_bridge_add_typed() takes its own reference to the
panel via drm_panel_get(), does this code leak the initial reference
returned by of_drm_find_panel() since drm_panel_put() is never called?

This same leak pattern appears to exist in other callers as well, such as
samsung-dsim.c:samsung_dsim_host_attach() and
rcar_du_encoder.c:rcar_du_encoder_init().

>  static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode)
>  {
>  	struct drm_panel *panel;
> @@ -554,6 +560,7 @@ static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle *fwnode
>  
>  	list_for_each_entry(panel, &panel_list, list) {
>  		if (dev_fwnode(panel->dev) == fwnode) {
> +			drm_panel_get(panel);
>  			mutex_unlock(&panel_lock);

[Severity: High]
This is a pre-existing issue, but is there a time-of-check to time-of-use
race condition when callers attempt to pin the underlying device?

When find_panel_by_fwnode() (or find_panel_by_dev()) returns, the panel's
internal refcount is incremented under panel_lock, but the underlying
struct device (panel->dev) is not pinned.

If a concurrent unbind occurs before the caller pins the device, the device
memory could be freed. For example, looking at drm_panel_add_follower():

drivers/gpu/drm/drm_panel.c:drm_panel_add_follower() {
    panel = find_panel_by_dev(follower_dev);
    if (IS_ERR(panel))
        return PTR_ERR(panel);

    get_device(panel->dev);
}

If the panel device driver unbinds immediately after panel_lock is dropped,
could get_device(panel->dev) trigger a use-after-free on the destroyed
device?

>  			return panel;
>  		}

[ ... ]

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