linux-kernel - [PATCH] driver core: get_dev_from_fwnode(): document potential race

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20250829205911.33142-1-dakr@kernel.org>
Date: Fri, 29 Aug 2025 22:58:57 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: gregkh@...uxfoundation.org,
	rafael@...nel.org
Cc: linux-kernel@...r.kernel.org,
	Danilo Krummrich <dakr@...nel.org>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Saravana Kannan <saravanak@...gle.com>
Subject: [PATCH] driver core: get_dev_from_fwnode(): document potential race

Commit 9a4681a485ee ("driver core: Export get_dev_from_fwnode()") made
get_dev_from_fwnode() publicly available, but didn't document the
guarantees a caller must uphold:

get_dev_from_fwnode() obtains a reference count from the device pointer
stored in a struct fwnode_handle. While having its own reference count,
struct fwnode_handle does not keep a reference count of the device it
has a pointer to.

Consequently, a caller must guarantee that it is impossible that the
last device reference is dropped and the device is released concurrently
while calling get_dev_from_fwnode(), otherwise this is a potential UAF
and hence a bug.

Thus, document this potential race condition for get_dev_from_fwnode().

Cc: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Saravana Kannan <saravanak@...gle.com>
Signed-off-by: Danilo Krummrich <dakr@...nel.org>
---
Another note on this:

It seems that (outside of drivers/base/core.c) this is only used by

  - of_genpd_add_provider_simple()
  - of_genpd_add_provider_onecell()

At a first glance, I can't see any caller of those two functions that is not
eventually coming from probe(), i.e. a context where we already have a valid
device pointer.

So, it seems to me that it would have been the much safer choice to make the two
functions above take a struct device * as argument directly, instead of making
get_dev_from_fwnode() publicly available, just to recover a device reference
count from a struct fwnode_handle.
---
 drivers/base/core.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index d22d6b23e758..cd806be435cd 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -5278,6 +5278,25 @@ void device_set_node(struct device *dev, struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(device_set_node);
 
+/**
+ * get_dev_from_fwnode - Obtain a reference count of the struct device the
+ * struct fwnode_handle is associated with.
+ * @fwnode: The pointer to the struct fwnode_handle to obtain the struct device
+ * reference count of.
+ *
+ * This function obtains a reference count of the device the device pointer
+ * embedded in the struct fwnode_handle points to.
+ *
+ * Note that the struct device pointer embedded in struct fwnode_handle does
+ * *not* have a reference count of the struct device itself.
+ *
+ * Hence, it is a UAF (and thus a bug) to call this function if the caller can't
+ * guarantee that the last reference count of the corresponding struct device is
+ * not dropped concurrently.
+ *
+ * This is possible since struct fwnode_handle has its own reference count and
+ * hence can out-live the struct device it is associated with.
+ */
 struct device *get_dev_from_fwnode(struct fwnode_handle *fwnode)
 {
 	return get_device((fwnode)->dev);

base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
-- 
2.51.0