diff mbox series

[v3,4/4] drm/panel: find_panel_by_fwnode() return a counted reference

Message ID 20260717-drm_refcount_wiring-v3-4-023900c32e01@redhat.com (mailing list archive)
State New
Headers
Series drm/panel: refcounting panel lookups and references |

Commit Message

Albert Esteve July 17, 2026, 2:02 p.m. UTC 
find_panel_by_fwnode() is the fwnode-based counterpart to
of_drm_find_panel(), used internally by drm_panel_add_follower().
Like of_drm_find_panel(), it returned an unrefcounted pointer,
leaving a window where the panel could be freed between the lookup
and first use.

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.

Apply the same fix: call drm_panel_get() under panel_lock before
returning. Since find_panel_by_fwnode() now transfers a counted
reference to drm_panel_add_follower(), drm_panel_remove_follower()
must balance it with a matching drm_panel_put().

Acked-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 drivers/gpu/drm/drm_panel.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org July 17, 2026, 2:14 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] TOCTOU race condition in `drm_panel_add_follower` leading to Use-After-Free of `panel->dev`.
--

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

drm/panel: find_panel_by_fwnode() return a counted reference

This patch updates find_panel_by_fwnode() to return a reference-counted
drm_panel pointer, preventing the panel's container memory from being
released prematurely. It also updates drm_panel_remove_follower() to
balance the reference with drm_panel_put().

> 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
[ ... ]
> @@ -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 does releasing the panel_lock here leave
a TOCTOU race window where panel->dev can be freed before it's used by the
caller?

If the panel's device is unregistered concurrently (e.g., via sysfs unbind
of its parent), the following sequence seems possible:

CPU1 in find_panel_by_fwnode():
    drm_panel_get(panel);
    mutex_unlock(&panel_lock);
    return panel;

CPU2 concurrently unregisters the device:
    drm_panel_remove() runs, removing the panel from panel_list.
    The device's reference count drops to zero and the struct device behind
    panel->dev is freed.

CPU1 in drm_panel_add_follower():
    get_device(panel->dev);

Since panel->dev now points to a freed device, could this cause a
use-after-free?

>  			return panel;
>  		}
>  	}
Luca Ceresoli July 20, 2026, 1:13 p.m. UTC | #2 
Hello Albert,

On Fri Jul 17, 2026 at 4:02 PM CEST, Albert Esteve wrote:
> find_panel_by_fwnode() is the fwnode-based counterpart to
> of_drm_find_panel(), used internally by drm_panel_add_follower().
> Like of_drm_find_panel(), it returned an unrefcounted pointer,
> leaving a window where the panel could be freed between the lookup
> and first use.
>
> 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.
>
> Apply the same fix: call drm_panel_get() under panel_lock before
> returning. Since find_panel_by_fwnode() now transfers a counted
> reference to drm_panel_add_follower(), drm_panel_remove_follower()
> must balance it with a matching drm_panel_put().
>
> Acked-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
>  drivers/gpu/drm/drm_panel.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> 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.
> + */
>  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);
>  			return panel;
>  		}
> @@ -690,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);

Based on your description, can we drop the get_device/put_device() now?

That can be done as a future cleanup, so:
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
diff mbox series

Patch

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.
+ */
 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);
 			return panel;
 		}
@@ -690,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);