[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.

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

[Greg Kroah-Hartman]

On Fri, May 08, 2026 at 02:03:44AM +0200, Danilo Krummrich wrote:
> 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.
> 

Sure, but for now I'll go take this one.

thanks,

greg k-h

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

[Danilo Krummrich]

On 5/22/26 12:24 PM, Greg Kroah-Hartman wrote:
> Sure, but for now I'll go take this one.
The follow-up commit 7eba000621ff ("device property: initialize the remaining
fields of fwnode_handle in fwnode_init()") is already in driver-core-next.

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

[Danilo Krummrich]

On 5/22/26 12:40 PM, Danilo Krummrich wrote:
> On 5/22/26 12:24 PM, Greg Kroah-Hartman wrote:
>> Sure, but for now I'll go take this one.
> The follow-up commit 7eba000621ff ("device property: initialize the remaining
> fields of fwnode_handle in fwnode_init()") is already in driver-core-next.

s/follow-up/v2/

https://lore.kernel.org/all/20260511074927.9473-1-bartosz.golaszewski@oss.qualcomm.com/

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

[Greg Kroah-Hartman]

On Fri, May 22, 2026 at 12:43:00PM +0200, Danilo Krummrich wrote:
> On 5/22/26 12:40 PM, Danilo Krummrich wrote:
> > On 5/22/26 12:24 PM, Greg Kroah-Hartman wrote:
> >> Sure, but for now I'll go take this one.
> > The follow-up commit 7eba000621ff ("device property: initialize the remaining
> > fields of fwnode_handle in fwnode_init()") is already in driver-core-next.
> 
> s/follow-up/v2/
> 
> https://lore.kernel.org/all/20260511074927.9473-1-bartosz.golaszewski@oss.qualcomm.com/
> 

Ugh, ok, we will have a merge conflict, but at least the bugfix will
propagate to stable trees :)

thanks,

greg "digging out from a huge email backlog" k-h

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

[Bartosz Golaszewski]

On Fri, May 22, 2026 at 12:52 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, May 22, 2026 at 12:43:00PM +0200, Danilo Krummrich wrote:
> > On 5/22/26 12:40 PM, Danilo Krummrich wrote:
> > > On 5/22/26 12:24 PM, Greg Kroah-Hartman wrote:
> > >> Sure, but for now I'll go take this one.
> > > The follow-up commit 7eba000621ff ("device property: initialize the remaining
> > > fields of fwnode_handle in fwnode_init()") is already in driver-core-next.
> >
> > s/follow-up/v2/
> >
> > https://lore.kernel.org/all/20260511074927.9473-1-bartosz.golaszewski@oss.qualcomm.com/
> >
>
> Ugh, ok, we will have a merge conflict, but at least the bugfix will
> propagate to stable trees :)
>
> thanks,
>
> greg "digging out from a huge email backlog" k-h

There's no need for you to queue this one, the v2 Danilo linked
contains this change and some more hardening on top for consistency.

Bart