On Mon, Jul 13, 2026 at 2:08 PM <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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().

Uh this is a real issue that I missed in several callers. I will add
drm_panel_put() after the bridge creation where needed in the next
version.

BR,
Albert.

>
> >  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/[email protected]?part=4
>

Reply via email to