[3/5] drm/panel: make *find_panel*() return a counted reference

Message ID 20260626-drm_refcount_wiring-v1-3-cca1a7b3bdef@redhat.com (mailing list archive)
State New
Headers
Series drm/panel: refcounting panel lookups and references |

Commit Message

Albert Esteve June 26, 2026, 12:03 p.m. UTC
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 <aesteve@redhat.com>
---
 drivers/gpu/drm/drm_panel.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)
  

Comments

Maxime Ripard June 26, 2026, 12:50 p.m. UTC | #1
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 <aesteve@redhat.com>
> ---
>  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

> @@ -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
  
Albert Esteve June 26, 2026, 3:11 p.m. UTC | #2
On Fri, Jun 26, 2026 at 2:50 PM Maxime Ripard <mripard@kernel.org> 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 <aesteve@redhat.com>
> > ---
> >  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
  

Patch

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;
 		}
@@ -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);