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

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.

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

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.

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/

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

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

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

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

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

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