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

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

[Danilo Krummrich]

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@linaro.org>
Cc: Saravana Kannan <saravanak@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.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