[PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[Bartosz Golaszewski]

If a firmware node is allocated on the stack (for instance: temporary
software node whose life-time we control) or on the heap - but using a
non-zeroing allocation function - and initialized using fwnode_init(),
its secondary pointer will contain uninitalized memory which likely will
be neither NULL nor IS_ERR() and so may end up being dereferenced (for
example: in dev_to_swnode()). Set fwnode->secondary to NULL on
initialization.

Cc: stable@vger.kernel.org
Fixes: 01bb86b380a3 ("driver core: Add fwnode_init()")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 include/linux/fwnode.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 80b38fbf2121..31df7608737e 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -208,6 +208,7 @@ struct fwnode_operations {
 static inline void fwnode_init(struct fwnode_handle *fwnode,
 			       const struct fwnode_operations *ops)
 {
+	fwnode->secondary = NULL;
 	fwnode->ops = ops;
 	INIT_LIST_HEAD(&fwnode->consumers);
 	INIT_LIST_HEAD(&fwnode->suppliers);
-- 
2.47.3

Re: [PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[Andy Shevchenko]

On Wed, May 06, 2026 at 01:57:00PM +0200, Bartosz Golaszewski wrote:
> If a firmware node is allocated on the stack (for instance: temporary
> software node whose life-time we control) or on the heap - but using a
> non-zeroing allocation function - and initialized using fwnode_init(),
> its secondary pointer will contain uninitalized memory which likely will
> be neither NULL nor IS_ERR() and so may end up being dereferenced (for
> example: in dev_to_swnode()). Set fwnode->secondary to NULL on
> initialization.

Hmm... Are you going to use that outside of fw_devlink?

The patch itself looks good to me, but I'm not sure I understand how it's
related to all the work you are doing WRT fwnode core implementation.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

But Saravana is the best person to actually tell if this patch makes sense.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[Bartosz Golaszewski]

On Wed, May 6, 2026 at 3:11 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, May 06, 2026 at 01:57:00PM +0200, Bartosz Golaszewski wrote:
> > If a firmware node is allocated on the stack (for instance: temporary
> > software node whose life-time we control) or on the heap - but using a
> > non-zeroing allocation function - and initialized using fwnode_init(),
> > its secondary pointer will contain uninitalized memory which likely will
> > be neither NULL nor IS_ERR() and so may end up being dereferenced (for
> > example: in dev_to_swnode()). Set fwnode->secondary to NULL on
> > initialization.
>
> Hmm... Are you going to use that outside of fw_devlink?
>
> The patch itself looks good to me, but I'm not sure I understand how it's
> related to all the work you are doing WRT fwnode core implementation.
>
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> But Saravana is the best person to actually tell if this patch makes sense.
>

I just stumbled upon this crash working on adding support for
fw_devlink for software nodes. I figured it makes sense fixing it even
if we have never hit it before.

Bart

Re: [PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[Rafael J. Wysocki]

On Wed, May 6, 2026 at 1:57 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> If a firmware node is allocated on the stack (for instance: temporary
> software node whose life-time we control) or on the heap - but using a
> non-zeroing allocation function - and initialized using fwnode_init(),
> its secondary pointer will contain uninitalized memory which likely will
> be neither NULL nor IS_ERR() and so may end up being dereferenced (for
> example: in dev_to_swnode()). Set fwnode->secondary to NULL on
> initialization.
>
> Cc: stable@vger.kernel.org
> Fixes: 01bb86b380a3 ("driver core: Add fwnode_init()")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
>  include/linux/fwnode.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 80b38fbf2121..31df7608737e 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -208,6 +208,7 @@ struct fwnode_operations {
>  static inline void fwnode_init(struct fwnode_handle *fwnode,
>                                const struct fwnode_operations *ops)
>  {
> +       fwnode->secondary = NULL;
>         fwnode->ops = ops;
>         INIT_LIST_HEAD(&fwnode->consumers);
>         INIT_LIST_HEAD(&fwnode->suppliers);
> --
> 2.47.3
>
>

Re: [PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[Sakari Ailus]

On Wed, May 06, 2026 at 01:57:00PM +0200, Bartosz Golaszewski wrote:
> If a firmware node is allocated on the stack (for instance: temporary
> software node whose life-time we control) or on the heap - but using a
> non-zeroing allocation function - and initialized using fwnode_init(),
> its secondary pointer will contain uninitalized memory which likely will
> be neither NULL nor IS_ERR() and so may end up being dereferenced (for
> example: in dev_to_swnode()). Set fwnode->secondary to NULL on
> initialization.
> 
> Cc: stable@vger.kernel.org
> Fixes: 01bb86b380a3 ("driver core: Add fwnode_init()")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus

Re: [PATCH] device property: set fwnode->secondary to NULL in fwnode_init()

[Danilo Krummrich]

On Wed May 6, 2026 at 1:57 PM CEST, Bartosz Golaszewski wrote:
> If a firmware node is allocated on the stack (for instance: temporary
> software node whose life-time we control) or on the heap - but using a
> non-zeroing allocation function - and initialized using fwnode_init(),
> its secondary pointer will contain uninitalized memory which likely will
> be neither NULL nor IS_ERR().

I see why secondary is generally more prone to this, but if the justification of
this change is to not rely on the caller to zero out the memory, then we might
just want to initialize all fields.

For instance, if the caller is allowed to not zero-initialize the memory then
having flags with a random value isn't correct either; all accessors are atomic
bitwise operations that never zero the whole field.