It has turned out get_dev_from_fwnode() is useful at a few other places
outside of the driver core, as in gpiolib.c for example. Therefore let's
make it available as a common helper function.

Suggested-by: Saravana Kannan <saravanak@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Hiago De Franco <hiago.franco@toradex.com> # Colibri iMX8X
Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/core.c    | 8 ++++++--
 include/linux/device.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index cbc0099d8ef2..6f91ece7c06a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1881,8 +1881,6 @@ static void fw_devlink_unblock_consumers(struct device *dev)
 	device_links_write_unlock();
 }
 
-#define get_dev_from_fwnode(fwnode)	get_device((fwnode)->dev)
-
 static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
 {
 	struct device *dev;
@@ -5281,6 +5279,12 @@ void device_set_node(struct device *dev, struct fwnode_handle *fwnode)
 }
 EXPORT_SYMBOL_GPL(device_set_node);
 
+struct device *get_dev_from_fwnode(struct fwnode_handle *fwnode)
+{
+	return get_device((fwnode)->dev);
+}
+EXPORT_SYMBOL_GPL(get_dev_from_fwnode);
+
 int device_match_name(struct device *dev, const void *name)
 {
 	return sysfs_streq(dev_name(dev), name);
diff --git a/include/linux/device.h b/include/linux/device.h
index 4940db137fff..315b00171335 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1048,6 +1048,7 @@ void device_set_node(struct device *dev, struct fwnode_handle *fwnode);
 int device_add_of_node(struct device *dev, struct device_node *of_node);
 void device_remove_of_node(struct device *dev);
 void device_set_of_node_from_dev(struct device *dev, const struct device *dev2);
+struct device *get_dev_from_fwnode(struct fwnode_handle *fwnode);
 
 static inline struct device_node *dev_of_node(struct device *dev)
 {
-- 
2.43.0

On Tue, Jul 01, 2025 at 01:47:19PM +0200, Ulf Hansson wrote:
> It has turned out get_dev_from_fwnode() is useful at a few other places
> outside of the driver core, as in gpiolib.c for example. Therefore let's
> make it available as a common helper function.
> 
> Suggested-by: Saravana Kannan <saravanak@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Tested-by: Hiago De Franco <hiago.franco@toradex.com> # Colibri iMX8X
> Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/core.c    | 8 ++++++--
>  include/linux/device.h | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

On Wed, Jul 02, 2025 at 09:34:12AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jul 01, 2025 at 01:47:19PM +0200, Ulf Hansson wrote:
> > It has turned out get_dev_from_fwnode() is useful at a few other places
> > outside of the driver core, as in gpiolib.c for example. Therefore let's
> > make it available as a common helper function.
> > 
> > Suggested-by: Saravana Kannan <saravanak@google.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Tested-by: Hiago De Franco <hiago.franco@toradex.com> # Colibri iMX8X
> > Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >  drivers/base/core.c    | 8 ++++++--
> >  include/linux/device.h | 1 +
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> > 
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I'm a bit concerned about exporting get_dev_from_fwnode() -- at least without a
clear note on that this helper should be used with caution.

AFAIK, a struct fwnode_handle instance does not have a reference count for its
struct device pointer.

Hence, calling get_dev_from_fwnode() with a valid fwnode handle is not enough.
The caller also needs to ensure that the device the fwnode has a pointer to has
not been released yet.

If this really needs to be exported, can we please add documentation covering
this properly?

- Danilo

On Wed, Jul 2, 2025 at 12:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed, Jul 02, 2025 at 09:34:12AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Jul 01, 2025 at 01:47:19PM +0200, Ulf Hansson wrote:
> > > It has turned out get_dev_from_fwnode() is useful at a few other places
> > > outside of the driver core, as in gpiolib.c for example. Therefore let's
> > > make it available as a common helper function.
> > >
> > > Suggested-by: Saravana Kannan <saravanak@google.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Tested-by: Hiago De Franco <hiago.franco@toradex.com> # Colibri iMX8X
> > > Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >  drivers/base/core.c    | 8 ++++++--
> > >  include/linux/device.h | 1 +
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> >
> > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> I'm a bit concerned about exporting get_dev_from_fwnode() -- at least without a
> clear note on that this helper should be used with caution.
>
> AFAIK, a struct fwnode_handle instance does not have a reference count for its
> struct device pointer.
>
> Hence, calling get_dev_from_fwnode() with a valid fwnode handle is not enough.

Not enough for what?

> The caller also needs to ensure that the device the fwnode has a pointer to has
> not been released yet.

Why? The point of the API is to give you the driver core's notion of
the primary device that corresponds to a fwnode at that instant.

There's no refcount needed for that. This is just a simpler way than
looping through all the devices to find the first device that has that
specific fwnode.

It's only the device that needs to be ref counted because the caller
needs to do stuff with it.

-Saravana

On Wed, Jul 02, 2025 at 02:34:04PM -0700, Saravana Kannan wrote:
> On Wed, Jul 2, 2025 at 12:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Wed, Jul 02, 2025 at 09:34:12AM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Jul 01, 2025 at 01:47:19PM +0200, Ulf Hansson wrote:
> > > > It has turned out get_dev_from_fwnode() is useful at a few other places
> > > > outside of the driver core, as in gpiolib.c for example. Therefore let's
> > > > make it available as a common helper function.
> > > >
> > > > Suggested-by: Saravana Kannan <saravanak@google.com>
> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Tested-by: Hiago De Franco <hiago.franco@toradex.com> # Colibri iMX8X
> > > > Tested-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> # TI AM62A,Xilinx ZynqMP ZCU106
> > > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > ---
> > > >  drivers/base/core.c    | 8 ++++++--
> > > >  include/linux/device.h | 1 +
> > > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > > >
> > >
> > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > I'm a bit concerned about exporting get_dev_from_fwnode() -- at least without a
> > clear note on that this helper should be used with caution.
> >
> > AFAIK, a struct fwnode_handle instance does not have a reference count for its
> > struct device pointer.
> >
> > Hence, calling get_dev_from_fwnode() with a valid fwnode handle is not enough.
> 
> Not enough for what?

Having a valid pointer to a fwnode does not guarantee that fwnode->dev is a
valid pointer. Given that fwnode is reference counted itself, but only has a
weak reference of the device behind fwnode->dev, the device may have been
released already.

If the scope this function is called from can't guarantee that fwnode->dev has
not been released yet, it's a potential UAF.

Yes, device_del() sets dev->fwnode->dev = NULL. But that makes it still racy.

If someone has a reference count on the fwnode and calls get_dev_from_fwnode()
while device_del() runs concurrently (assuming that device_del() drops the last
reference of the device), it's a race with a potential UAF.

We should warn about this, when makeing get_dev_from_fwnode() and API that can
be used by *everyone*.

- Danilo