On Fri, Jun 26, 2026 at 2:50 PM Maxime Ripard <[email protected]> wrote: > > On Fri, Jun 26, 2026 at 02:03:25PM +0200, Albert Esteve wrote: > > Callers of of_drm_find_panel() receive a pointer with no reference > > held, creating a window where the panel device can be unregistered > > and freed between the lookup and first use (e.g., drm_panel_prepare()). > > > > find_panel_by_fwnode() is the fwnode counterpart of of_drm_find_panel(). > > drm_panel_add_follower() worked around the missing panel kref by calling > > get_device() on the panel's underlying struct device. However, get_device() > > only prevents the device kobject from being freed. It does not prevent the > > panel's kzalloc()'d container memory from being released when the kref > > reaches zero. > > > > Fix both lookup functions by acquiring a reference with drm_panel_get() > > before returning, under panel_lock. Callers are now responsible for calling > > drm_panel_put() when they no longer need the pointer. > > > > Signed-off-by: Albert Esteve <[email protected]> > > --- > > drivers/gpu/drm/drm_panel.c | 22 +++++++++++++++++----- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > > index 545fe93dc28fe..a00ae98ed0956 100644 > > --- a/drivers/gpu/drm/drm_panel.c > > +++ b/drivers/gpu/drm/drm_panel.c > > @@ -458,14 +458,17 @@ EXPORT_SYMBOL(__devm_drm_panel_alloc); > > > > #ifdef CONFIG_OF > > /** > > - * of_drm_find_panel - look up a panel using a device tree node > > + * of_drm_find_panel - look up and reference a panel by device tree node > > * @np: device tree node of the panel > > * > > * Searches the set of registered panels for one that matches the given > > device > > - * tree node. If a matching panel is found, return a pointer to it. > > + * tree node. If a matching panel is found, the panel's reference count is > > + * incremented before returning a pointer to it. The caller must call > > + * drm_panel_put() when it no longer needs the panel pointer. > > * > > - * Return: A pointer to the panel registered for the specified device tree > > - * node or an ERR_PTR() if no panel matching the device tree node can be > > found. > > + * Return: A reference-counted pointer to the panel registered for the > > specified > > + * device tree node or an ERR_PTR() if no panel matching the device tree > > node > > + * can be found. > > * > > * Possible error codes returned by this function: > > * > > @@ -484,6 +487,7 @@ struct drm_panel *of_drm_find_panel(const struct > > device_node *np) > > > > list_for_each_entry(panel, &panel_list, list) { > > if (panel->dev->of_node == np) { > > + drm_panel_get(panel); > > mutex_unlock(&panel_lock); > > return panel; > > } > > @@ -538,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. > > + */ > > static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle > > *fwnode) > > { > > struct drm_panel *panel; > > @@ -550,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); > > return panel; > > } > > This part should probably be in a separate patch
Yes. This is another place where I hesitated on organization, as it is very similar to of_drm_find_panel() fix. But find_panel_by_fwnode() is much more self-contained (it is declared static to begin with). So it makes sense to split them. I will do so in the next version. > > > @@ -686,6 +697,7 @@ void drm_panel_remove_follower(struct > > drm_panel_follower *follower) > > mutex_unlock(&panel->follower_lock); > > > > put_device(panel->dev); > > + drm_panel_put(panel); > > } > > EXPORT_SYMBOL(drm_panel_remove_follower); > > together with this one? > > Maxime
