[PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[mike.isely]

From: Mike Isely <mike.isely@cobaltdigital.com>

Correct issue in drivers/base/swnode.c that can lead to use-after-free
due to kobject reference counting error, which itself is due to
incorrect behavior with the "managed" struct swnode flag in
circumstances involving child struct device instances where the parent
struct device is managing a struct swnode.

Use-after-free in this case led to an Oops and a subsequent kernel
memory leak, but realistically it's kernel heap corruption, so any
manner of chaos can result, if left unaddressed.

This was detected in kernel 6.12, verified also in kernel 6.6.  Visual
inspection in 6.19.3 source (the latest as of right now) shows the
same issue.  The nearly trivial fix was verified in 6.12.  While this
patches against 6.19.3, IMHO this is a candidate for all LTS kernels.

Mike Isely (1):
  sofware node: Only the managing device can unreference managed
    software node

 drivers/base/swnode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.47.3

[PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[mike.isely]

From: Mike Isely <mike.isely@cobaltdigital.com>

A scenario exists where device_create_managed_software_node() is used
to create an swnode instance that will be implicitly shared to a child
device despite best intentions not to permit such sharing (per the
comment in device_create_managed_software_node()).  I encountered this
with the sfp kernel module when it was instantiated with properties
via a call to platform_device_register_full() - it will create hwmon
child devices which get all property references.  Unfortunately with
just a "managed" boolean in struct swnode handling this, then
kobject_put() gets called for the managed aspect when the child device
goes away instead of the parent.  This leads to premature freeing of
the swnode structure, followed by use-after-free problems, heap
corruption, and generally chaos / crashes / misbehavior in the kernel.

This commit changes that boolean into a pointer to the actual managing
struct device, which is then checked against the struct device
instance that is actually going away (via the usual call back into
software_node_notify_remove()).  Thus the child device removal is
ignored as it should, and we only do the kobject_put() when the actual
managing struct device instance goes away.  We effectively carry a
little bit more information now so that we can be sure to clean up
only when the correct struct device instance is actually going away.

Note that while we are now keeping a pointer to a struct device here,
this is safe to do because the pointer itself only stays in use while
the pointed-to device remains valid.  (So no need to be concerned
about additional reference counting.)

Signed-off-by: Mike Isely <isely@pobox.com>
---
 drivers/base/swnode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 16a8301c25d63..2ead8e4062f63 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -36,8 +36,8 @@ struct swnode {
 	struct list_head children;
 	struct swnode *parent;
 
+	struct device *managing_dev;
 	unsigned int allocated:1;
-	unsigned int managed:1;
 };
 
 static DEFINE_IDA(swnode_root_ids);
@@ -1078,7 +1078,7 @@ int device_create_managed_software_node(struct device *dev,
 	if (IS_ERR(fwnode))
 		return PTR_ERR(fwnode);
 
-	to_swnode(fwnode)->managed = true;
+	to_swnode(fwnode)->managing_dev = dev;
 	set_secondary_fwnode(dev, fwnode);
 
 	if (device_is_registered(dev))
@@ -1121,7 +1121,7 @@ void software_node_notify_remove(struct device *dev)
 	sysfs_remove_link(&dev->kobj, "software_node");
 	kobject_put(&swnode->kobj);
 
-	if (swnode->managed) {
+	if (swnode->managing_dev == dev) {
 		set_secondary_fwnode(dev, NULL);
 		kobject_put(&swnode->kobj);
 	}
-- 
2.47.3

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:
> From: Mike Isely <mike.isely@cobaltdigital.com>

Author's and...

> A scenario exists where device_create_managed_software_node() is used
> to create an swnode instance that will be implicitly shared to a child
> device despite best intentions not to permit such sharing (per the
> comment in device_create_managed_software_node()).  I encountered this
> with the sfp kernel module when it was instantiated with properties

SFP? Or is it the name of the actual module in the kernel?

> via a call to platform_device_register_full() - it will create hwmon
> child devices which get all property references.  Unfortunately with
> just a "managed" boolean in struct swnode handling this, then
> kobject_put() gets called for the managed aspect when the child device
> goes away instead of the parent.  This leads to premature freeing of
> the swnode structure, followed by use-after-free problems, heap
> corruption, and generally chaos / crashes / misbehavior in the kernel.
> 
> This commit changes that boolean into a pointer to the actual managing
> struct device, which is then checked against the struct device
> instance that is actually going away (via the usual call back into
> software_node_notify_remove()).  Thus the child device removal is
> ignored as it should, and we only do the kobject_put() when the actual
> managing struct device instance goes away.  We effectively carry a
> little bit more information now so that we can be sure to clean up
> only when the correct struct device instance is actually going away.
> 
> Note that while we are now keeping a pointer to a struct device here,
> this is safe to do because the pointer itself only stays in use while
> the pointed-to device remains valid.  (So no need to be concerned
> about additional reference counting.)

The term is called "object lifetime".

> Signed-off-by: Mike Isely <isely@pobox.com>

...submitter's addresses are different. Either it should be send from the
mentioned address, or you should have

  From: Author <...>
  ...

  SoB: Author <...>
  SoB: Submitter <...>

...

What about the use case (don't know if it's pure theoretical or practical)
when there is a parent and a few children and the managed swnode appears
in one of the children? With some other dependencies between children
it might affect how swnode is get cleaned up. Simple and regular approach
is to cleanup children in the reversed order, but I can't say that it's
always the case. Some children in some corner cases might have their own
dependencies (I saw some strange devices or device drivers where the HW
is a bit complicated and driver is written without much care).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Mike Isely]

On Wed, 25 Feb 2026, Andy Shevchenko wrote:

> On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:
> > From: Mike Isely <mike.isely@cobaltdigital.com>
> 
> Author's and...
> 
> > A scenario exists where device_create_managed_software_node() is used
> > to create an swnode instance that will be implicitly shared to a child
> > device despite best intentions not to permit such sharing (per the
> > comment in device_create_managed_software_node()).  I encountered this
> > with the sfp kernel module when it was instantiated with properties
> 
> SFP? Or is it the name of the actual module in the kernel?

Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
the piece of hardware it works with, an SFP cage.  This is logic which 
monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
When a transceiver appears, the sfp kernel module will create a child 
hwmon device instance to monitor various bits of metadata from the 
transceiver.  When that transceiver goes away, the sfp kernel module 
will tear down that child hwmon device instance.

The sfp kernel module needs resources configured to know where to 
monitor; in our case that is set up dynamically by another locally 
written kernel module (which iteracts with an FPGA we have where the SFP 
hardware elements reside), and that kernel module will combine 
devicetree information with some run-time information to generate the 
properties handed off to the sfp kernel module instantiation.  We use 
platform_device_register_full() to instantiate it, and that in turn 
causes the swnode instance to be created.  The hwmon child instance 
later created by the sfp module ends up inheriting those resources, 
including the swnode instance, from the sfp parent device, and when the 
hwmon instance is later torn down, we end up with the use-after-free 
problem due to the swnode instance's reference count being incorrectly 
decremented by that child device due to the managed flag being set.

If the term SFP is unfamiliar, an explanation can be found here:

https://en.wikipedia.org/wiki/Small_Form-factor_Pluggable


> 
> > via a call to platform_device_register_full() - it will create hwmon
> > child devices which get all property references.  Unfortunately with
> > just a "managed" boolean in struct swnode handling this, then
> > kobject_put() gets called for the managed aspect when the child device
> > goes away instead of the parent.  This leads to premature freeing of
> > the swnode structure, followed by use-after-free problems, heap
> > corruption, and generally chaos / crashes / misbehavior in the kernel.
> > 
> > This commit changes that boolean into a pointer to the actual managing
> > struct device, which is then checked against the struct device
> > instance that is actually going away (via the usual call back into
> > software_node_notify_remove()).  Thus the child device removal is
> > ignored as it should, and we only do the kobject_put() when the actual
> > managing struct device instance goes away.  We effectively carry a
> > little bit more information now so that we can be sure to clean up
> > only when the correct struct device instance is actually going away.
> > 
> > Note that while we are now keeping a pointer to a struct device here,
> > this is safe to do because the pointer itself only stays in use while
> > the pointed-to device remains valid.  (So no need to be concerned
> > about additional reference counting.)
> 
> The term is called "object lifetime".

Meh


> 
> > Signed-off-by: Mike Isely <isely@pobox.com>
> 
> ...submitter's addresses are different. Either it should be send from the
> mentioned address, or you should have
> 
>   From: Author <...>
>   ...
> 
>   SoB: Author <...>
>   SoB: Submitter <...>
> 
> ...

This isn't the first time I've submitted patches.

The reason for SoB being isely@pobox.com is because that is my primary 
email address for which I've been known by in the kernel community.  
It's an address that is stable and has been associated with me since 
1996.  This address has a history and is associated with patches in the 
past going back a long ways.

The from-address of mike.isely@cobaltdigital.com is my current work 
email, which is associated with the reason for the patch.  My employment 
with Cobalt Digital goes back just a few years but my history with the 
Linux kernel goes back significantly further.

It's actually been a while since I sent in a patch.  Last time I did, 
this is how I did it and it wasn't a problem then.  I don't recall 
seeing anything that required the from-address to equate to the SoB.  
Seems like a SoB associated with a long-term known stable email is 
better than something more ephemeral.  If a requirement of equating the 
two is the case now, then I apologize.


> 
> What about the use case (don't know if it's pure theoretical or practical)
> when there is a parent and a few children and the managed swnode appears
> in one of the children? With some other dependencies between children
> it might affect how swnode is get cleaned up. Simple and regular approach
> is to cleanup children in the reversed order, but I can't say that it's
> always the case. Some children in some corner cases might have their own
> dependencies (I saw some strange devices or device drivers where the HW
> is a bit complicated and driver is written without much care).

Though I don't think that's practical, it should not be an issue either.  
Bottom line, swnode instances are reference-counted by virtue of 
containing a kobject instance within.  So even if a managing child goes 
away first, the reference count is still non-zero due to the reference 
from the parent.  So no risk of dangling pointers / use-after-free.  
Note also that management of object lifetime via reference counting here 
provides a fair amount of resilience against potential cleanup ordering 
issues.

An earlier attempt at a solution I had tried was to simply dispense with 
the notion of "managed" here since the reference counting is already 
going to ensure cleanup when the device goes away.  "Managed" in the 
context of struct device are for resources that need automatic cleanup 
which otherwise wouldn't already happen.  So it shouldn't be strictly 
even needed here.  However that didn't work due to a practical 
constraint: The swnode instance is created before the "managing" device 
instance is ready to establish a reference to the swnode instance.  
Turns out that the solution I ultimately hit upon is trivial.

From a standpoint of design hygiene, it makes logical sense that the 
entity which asserts "management" over something should be the same 
entity that deasserts it.  That's really all that this patch does.

  -Mike
   isely@pobox.com
   mike.isely@cobaltdigital.com

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:

...

> > > A scenario exists where device_create_managed_software_node() is used
> > > to create an swnode instance that will be implicitly shared to a child
> > > device despite best intentions not to permit such sharing (per the
> > > comment in device_create_managed_software_node()).  I encountered this
> > > with the sfp kernel module when it was instantiated with properties
> > 
> > SFP? Or is it the name of the actual module in the kernel?
> 
> Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> the piece of hardware it works with, an SFP cage.  This is logic which 
> monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> When a transceiver appears, the sfp kernel module will create a child 
> hwmon device instance to monitor various bits of metadata from the 
> transceiver.  When that transceiver goes away, the sfp kernel module 
> will tear down that child hwmon device instance.
> 
> The sfp kernel module needs resources configured to know where to 
> monitor; in our case that is set up dynamically by another locally 
> written kernel module (which iteracts with an FPGA we have where the SFP 
> hardware elements reside), and that kernel module will combine 

> devicetree information with some run-time information to generate the 
> properties handed off to the sfp kernel module instantiation.

What runtime information? Why this can't be done via DT overlay as others do?

> We use platform_device_register_full() to instantiate it, and that in turn 
> causes the swnode instance to be created.  The hwmon child instance 
> later created by the sfp module ends up inheriting those resources, 
> including the swnode instance, from the sfp parent device, and when the 
> hwmon instance is later torn down, we end up with the use-after-free 
> problem due to the swnode instance's reference count being incorrectly 
> decremented by that child device due to the managed flag being set.
> 
> If the term SFP is unfamiliar, an explanation can be found here:
> 
> https://en.wikipedia.org/wiki/Small_Form-factor_Pluggable
> 
> > > via a call to platform_device_register_full() - it will create hwmon
> > > child devices which get all property references.  Unfortunately with
> > > just a "managed" boolean in struct swnode handling this, then
> > > kobject_put() gets called for the managed aspect when the child device
> > > goes away instead of the parent.  This leads to premature freeing of
> > > the swnode structure, followed by use-after-free problems, heap
> > > corruption, and generally chaos / crashes / misbehavior in the kernel.
> > > 
> > > This commit changes that boolean into a pointer to the actual managing
> > > struct device, which is then checked against the struct device
> > > instance that is actually going away (via the usual call back into
> > > software_node_notify_remove()).  Thus the child device removal is
> > > ignored as it should, and we only do the kobject_put() when the actual
> > > managing struct device instance goes away.  We effectively carry a
> > > little bit more information now so that we can be sure to clean up
> > > only when the correct struct device instance is actually going away.
> > > 
> > > Note that while we are now keeping a pointer to a struct device here,
> > > this is safe to do because the pointer itself only stays in use while
> > > the pointed-to device remains valid.  (So no need to be concerned
> > > about additional reference counting.)
> > 
> > The term is called "object lifetime".
> 
> Meh
> 
> > > Signed-off-by: Mike Isely <isely@pobox.com>
> > 
> > ...submitter's addresses are different. Either it should be send from the
> > mentioned address, or you should have
> > 
> >   From: Author <...>
> >   ...
> > 
> >   SoB: Author <...>
> >   SoB: Submitter <...>
> > 
> > ...
> 
> This isn't the first time I've submitted patches.
> 
> The reason for SoB being isely@pobox.com is because that is my primary 
> email address for which I've been known by in the kernel community.  
> It's an address that is stable and has been associated with me since 
> 1996.  This address has a history and is associated with patches in the 
> past going back a long ways.
> 
> The from-address of mike.isely@cobaltdigital.com is my current work 
> email, which is associated with the reason for the patch.  My employment 
> with Cobalt Digital goes back just a few years but my history with the 
> Linux kernel goes back significantly further.
> 
> It's actually been a while since I sent in a patch.  Last time I did, 
> this is how I did it and it wasn't a problem then.

This is standard check in Linux Next, missed SoB by submitter (same
exists with committer). It's all documented and I'm surprised nobody
told you that before.

> I don't recall 
> seeing anything that required the from-address to equate to the SoB.  

From goes to Author field into Git, Author must provide a DCO. This all
is being required from day 1.

> Seems like a SoB associated with a long-term known stable email is 
> better than something more ephemeral.  If a requirement of equating the 
> two is the case now, then I apologize.

Documentation/process/submitting-patches.rst:

"Note, the From: tag is optional when the From: author is also the person (and
email) listed in the From: line of the email header."

There are also examples

"""
        From: From Author <from@author.example.org>

        <changelog>

        Co-developed-by: Random Co-Author <random@coauthor.example.org>
        Signed-off-by: Random Co-Author <random@coauthor.example.org>
        Signed-off-by: From Author <from@author.example.org>
        Co-developed-by: Submitting Co-Author <sub@coauthor.example.org>
        Signed-off-by: Submitting Co-Author <sub@coauthor.example.org>
"""

Does it help?

> > What about the use case (don't know if it's pure theoretical or practical)
> > when there is a parent and a few children and the managed swnode appears
> > in one of the children? With some other dependencies between children
> > it might affect how swnode is get cleaned up. Simple and regular approach
> > is to cleanup children in the reversed order, but I can't say that it's
> > always the case. Some children in some corner cases might have their own
> > dependencies (I saw some strange devices or device drivers where the HW
> > is a bit complicated and driver is written without much care).
> 
> Though I don't think that's practical, it should not be an issue either.  
> Bottom line, swnode instances are reference-counted by virtue of 
> containing a kobject instance within.  So even if a managing child goes 
> away first, the reference count is still non-zero due to the reference 
> from the parent.  So no risk of dangling pointers / use-after-free.  
> Note also that management of object lifetime via reference counting here 
> provides a fair amount of resilience against potential cleanup ordering 
> issues.
> 
> An earlier attempt at a solution I had tried was to simply dispense with 
> the notion of "managed" here since the reference counting is already 
> going to ensure cleanup when the device goes away.  "Managed" in the 
> context of struct device are for resources that need automatic cleanup 
> which otherwise wouldn't already happen.  So it shouldn't be strictly 
> even needed here.  However that didn't work due to a practical 
> constraint: The swnode instance is created before the "managing" device 
> instance is ready to establish a reference to the swnode instance.  
> Turns out that the solution I ultimately hit upon is trivial.
> 
> From a standpoint of design hygiene, it makes logical sense that the 
> entity which asserts "management" over something should be the same 
> entity that deasserts it.  That's really all that this patch does.

OK. Thanks for elaboration.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Mike Isely]

On Wed, 25 Feb 2026, Andy Shevchenko wrote:

> On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:
> 
> ...
> 
> > > > A scenario exists where device_create_managed_software_node() is used
> > > > to create an swnode instance that will be implicitly shared to a child
> > > > device despite best intentions not to permit such sharing (per the
> > > > comment in device_create_managed_software_node()).  I encountered this
> > > > with the sfp kernel module when it was instantiated with properties
> > > 
> > > SFP? Or is it the name of the actual module in the kernel?
> > 
> > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> > the piece of hardware it works with, an SFP cage.  This is logic which 
> > monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> > When a transceiver appears, the sfp kernel module will create a child 
> > hwmon device instance to monitor various bits of metadata from the 
> > transceiver.  When that transceiver goes away, the sfp kernel module 
> > will tear down that child hwmon device instance.
> > 
> > The sfp kernel module needs resources configured to know where to 
> > monitor; in our case that is set up dynamically by another locally 
> > written kernel module (which iteracts with an FPGA we have where the SFP 
> > hardware elements reside), and that kernel module will combine 
> 
> > devicetree information with some run-time information to generate the 
> > properties handed off to the sfp kernel module instantiation.
> 
> What runtime information? Why this can't be done via DT overlay as others do?

I don't recall the specifics.  It might be calculation of a unit name.  
The connectivity in this case is I2C so that should be a constant.  We 
have some variants where the FPGA is PCIE-connected to the host and so 
the memory map is a run-time calculation.  We have other drivers that 
have to be instantiated with run-time computed properties.  So we handle 
this as a general case.


> 
> > We use platform_device_register_full() to instantiate it, and that in turn 
> > causes the swnode instance to be created.  The hwmon child instance 
> > later created by the sfp module ends up inheriting those resources, 
> > including the swnode instance, from the sfp parent device, and when the 
> > hwmon instance is later torn down, we end up with the use-after-free 
> > problem due to the swnode instance's reference count being incorrectly 
> > decremented by that child device due to the managed flag being set.
> > 
> > If the term SFP is unfamiliar, an explanation can be found here:
> > 
> > https://en.wikipedia.org/wiki/Small_Form-factor_Pluggable
> > 
> > > > via a call to platform_device_register_full() - it will create hwmon
> > > > child devices which get all property references.  Unfortunately with
> > > > just a "managed" boolean in struct swnode handling this, then
> > > > kobject_put() gets called for the managed aspect when the child device
> > > > goes away instead of the parent.  This leads to premature freeing of
> > > > the swnode structure, followed by use-after-free problems, heap
> > > > corruption, and generally chaos / crashes / misbehavior in the kernel.
> > > > 
> > > > This commit changes that boolean into a pointer to the actual managing
> > > > struct device, which is then checked against the struct device
> > > > instance that is actually going away (via the usual call back into
> > > > software_node_notify_remove()).  Thus the child device removal is
> > > > ignored as it should, and we only do the kobject_put() when the actual
> > > > managing struct device instance goes away.  We effectively carry a
> > > > little bit more information now so that we can be sure to clean up
> > > > only when the correct struct device instance is actually going away.
> > > > 
> > > > Note that while we are now keeping a pointer to a struct device here,
> > > > this is safe to do because the pointer itself only stays in use while
> > > > the pointed-to device remains valid.  (So no need to be concerned
> > > > about additional reference counting.)
> > > 
> > > The term is called "object lifetime".
> > 
> > Meh
> > 
> > > > Signed-off-by: Mike Isely <isely@pobox.com>
> > > 
> > > ...submitter's addresses are different. Either it should be send from the
> > > mentioned address, or you should have
> > > 
> > >   From: Author <...>
> > >   ...
> > > 
> > >   SoB: Author <...>
> > >   SoB: Submitter <...>
> > > 
> > > ...
> > 
> > This isn't the first time I've submitted patches.
> > 
> > The reason for SoB being isely@pobox.com is because that is my primary 
> > email address for which I've been known by in the kernel community.  
> > It's an address that is stable and has been associated with me since 
> > 1996.  This address has a history and is associated with patches in the 
> > past going back a long ways.
> > 
> > The from-address of mike.isely@cobaltdigital.com is my current work 
> > email, which is associated with the reason for the patch.  My employment 
> > with Cobalt Digital goes back just a few years but my history with the 
> > Linux kernel goes back significantly further.
> > 
> > It's actually been a while since I sent in a patch.  Last time I did, 
> > this is how I did it and it wasn't a problem then.
> 
> This is standard check in Linux Next, missed SoB by submitter (same
> exists with committer). It's all documented and I'm surprised nobody
> told you that before.
> 
> > I don't recall 
> > seeing anything that required the from-address to equate to the SoB.  
> 
> From goes to Author field into Git, Author must provide a DCO. This all
> is being required from day 1.
> 
> > Seems like a SoB associated with a long-term known stable email is 
> > better than something more ephemeral.  If a requirement of equating the 
> > two is the case now, then I apologize.
> 
> Documentation/process/submitting-patches.rst:
> 
> "Note, the From: tag is optional when the From: author is also the person (and
> email) listed in the From: line of the email header."
> 
> There are also examples
> 
> """
>         From: From Author <from@author.example.org>
> 
>         <changelog>
> 
>         Co-developed-by: Random Co-Author <random@coauthor.example.org>
>         Signed-off-by: Random Co-Author <random@coauthor.example.org>
>         Signed-off-by: From Author <from@author.example.org>
>         Co-developed-by: Submitting Co-Author <sub@coauthor.example.org>
>         Signed-off-by: Submitting Co-Author <sub@coauthor.example.org>
> """
> 
> Does it help?

Would SoB lines by *both* addresses (even though it's the same person) 
clear that hurdle?

So noted.


> 
> > > What about the use case (don't know if it's pure theoretical or practical)
> > > when there is a parent and a few children and the managed swnode appears
> > > in one of the children? With some other dependencies between children
> > > it might affect how swnode is get cleaned up. Simple and regular approach
> > > is to cleanup children in the reversed order, but I can't say that it's
> > > always the case. Some children in some corner cases might have their own
> > > dependencies (I saw some strange devices or device drivers where the HW
> > > is a bit complicated and driver is written without much care).
> > 
> > Though I don't think that's practical, it should not be an issue either.  
> > Bottom line, swnode instances are reference-counted by virtue of 
> > containing a kobject instance within.  So even if a managing child goes 
> > away first, the reference count is still non-zero due to the reference 
> > from the parent.  So no risk of dangling pointers / use-after-free.  
> > Note also that management of object lifetime via reference counting here 
> > provides a fair amount of resilience against potential cleanup ordering 
> > issues.
> > 
> > An earlier attempt at a solution I had tried was to simply dispense with 
> > the notion of "managed" here since the reference counting is already 
> > going to ensure cleanup when the device goes away.  "Managed" in the 
> > context of struct device are for resources that need automatic cleanup 
> > which otherwise wouldn't already happen.  So it shouldn't be strictly 
> > even needed here.  However that didn't work due to a practical 
> > constraint: The swnode instance is created before the "managing" device 
> > instance is ready to establish a reference to the swnode instance.  
> > Turns out that the solution I ultimately hit upon is trivial.
> > 
> > From a standpoint of design hygiene, it makes logical sense that the 
> > entity which asserts "management" over something should be the same 
> > entity that deasserts it.  That's really all that this patch does.
> 
> OK. Thanks for elaboration.
> 
> 

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 02:16:39PM -0600, Mike Isely wrote:
> On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:

...

> > > > > A scenario exists where device_create_managed_software_node() is used
> > > > > to create an swnode instance that will be implicitly shared to a child
> > > > > device despite best intentions not to permit such sharing (per the
> > > > > comment in device_create_managed_software_node()).  I encountered this
> > > > > with the sfp kernel module when it was instantiated with properties
> > > > 
> > > > SFP? Or is it the name of the actual module in the kernel?
> > > 
> > > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> > > the piece of hardware it works with, an SFP cage.  This is logic which 
> > > monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> > > When a transceiver appears, the sfp kernel module will create a child 
> > > hwmon device instance to monitor various bits of metadata from the 
> > > transceiver.  When that transceiver goes away, the sfp kernel module 
> > > will tear down that child hwmon device instance.
> > > 
> > > The sfp kernel module needs resources configured to know where to 
> > > monitor; in our case that is set up dynamically by another locally 
> > > written kernel module (which iteracts with an FPGA we have where the SFP 
> > > hardware elements reside), and that kernel module will combine 
> > 
> > > devicetree information with some run-time information to generate the 
> > > properties handed off to the sfp kernel module instantiation.
> > 
> > What runtime information? Why this can't be done via DT overlay as others do?
> 
> I don't recall the specifics.  It might be calculation of a unit name.  
> The connectivity in this case is I2C so that should be a constant.  We 
> have some variants where the FPGA is PCIE-connected to the host and so 
> the memory map is a run-time calculation.  We have other drivers that 
> have to be instantiated with run-time computed properties.  So we handle 
> this as a general case.

But the configurations are semi-static, right? For the contents of FPGA we have
a specific manager that reloads the FPGA configuration.

Using swnode for dynamically calculated data seems weird. The data in swnodes
is usually static (const), I can't remember the case where we need to supply
run-time calculated values.

Since that, DT overlay approach seems fine, no?

> > > We use platform_device_register_full() to instantiate it, and that in turn 
> > > causes the swnode instance to be created.  The hwmon child instance 
> > > later created by the sfp module ends up inheriting those resources, 
> > > including the swnode instance, from the sfp parent device, and when the 
> > > hwmon instance is later torn down, we end up with the use-after-free 
> > > problem due to the swnode instance's reference count being incorrectly 
> > > decremented by that child device due to the managed flag being set.
> > > 
> > > If the term SFP is unfamiliar, an explanation can be found here:
> > > 
> > > https://en.wikipedia.org/wiki/Small_Form-factor_Pluggable
> > > 
> > > > > via a call to platform_device_register_full() - it will create hwmon
> > > > > child devices which get all property references.  Unfortunately with
> > > > > just a "managed" boolean in struct swnode handling this, then
> > > > > kobject_put() gets called for the managed aspect when the child device
> > > > > goes away instead of the parent.  This leads to premature freeing of
> > > > > the swnode structure, followed by use-after-free problems, heap
> > > > > corruption, and generally chaos / crashes / misbehavior in the kernel.
> > > > > 
> > > > > This commit changes that boolean into a pointer to the actual managing
> > > > > struct device, which is then checked against the struct device
> > > > > instance that is actually going away (via the usual call back into
> > > > > software_node_notify_remove()).  Thus the child device removal is
> > > > > ignored as it should, and we only do the kobject_put() when the actual
> > > > > managing struct device instance goes away.  We effectively carry a
> > > > > little bit more information now so that we can be sure to clean up
> > > > > only when the correct struct device instance is actually going away.
> > > > > 
> > > > > Note that while we are now keeping a pointer to a struct device here,
> > > > > this is safe to do because the pointer itself only stays in use while
> > > > > the pointed-to device remains valid.  (So no need to be concerned
> > > > > about additional reference counting.)
> > > > 
> > > > The term is called "object lifetime".
> > > 
> > > Meh

...

> Would SoB lines by *both* addresses (even though it's the same person)
> clear that hurdle?

Yes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Mike Isely]

On Thu, 26 Feb 2026, Andy Shevchenko wrote:

> On Wed, Feb 25, 2026 at 02:16:39PM -0600, Mike Isely wrote:
> > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:
> 
> ...
> 
> > > > > > A scenario exists where device_create_managed_software_node() is used
> > > > > > to create an swnode instance that will be implicitly shared to a child
> > > > > > device despite best intentions not to permit such sharing (per the
> > > > > > comment in device_create_managed_software_node()).  I encountered this
> > > > > > with the sfp kernel module when it was instantiated with properties
> > > > > 
> > > > > SFP? Or is it the name of the actual module in the kernel?
> > > > 
> > > > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> > > > the piece of hardware it works with, an SFP cage.  This is logic which 
> > > > monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> > > > When a transceiver appears, the sfp kernel module will create a child 
> > > > hwmon device instance to monitor various bits of metadata from the 
> > > > transceiver.  When that transceiver goes away, the sfp kernel module 
> > > > will tear down that child hwmon device instance.
> > > > 
> > > > The sfp kernel module needs resources configured to know where to 
> > > > monitor; in our case that is set up dynamically by another locally 
> > > > written kernel module (which iteracts with an FPGA we have where the SFP 
> > > > hardware elements reside), and that kernel module will combine 
> > > 
> > > > devicetree information with some run-time information to generate the 
> > > > properties handed off to the sfp kernel module instantiation.
> > > 
> > > What runtime information? Why this can't be done via DT overlay as others do?
> > 
> > I don't recall the specifics.  It might be calculation of a unit name.  
> > The connectivity in this case is I2C so that should be a constant.  We 
> > have some variants where the FPGA is PCIE-connected to the host and so 
> > the memory map is a run-time calculation.  We have other drivers that 
> > have to be instantiated with run-time computed properties.  So we handle 
> > this as a general case.
> 
> But the configurations are semi-static, right? For the contents of FPGA we have
> a specific manager that reloads the FPGA configuration.
> 
> Using swnode for dynamically calculated data seems weird. The data in swnodes
> is usually static (const), I can't remember the case where we need to supply
> run-time calculated values.

Semi-static means partially dynamic...

We use the kernel FPGA manager in one platform (zynqmp) and a userspace 
loader program in another (PCIE connected to bcm2711 soc).  However it 
is not reasonable to reload on-the-fly as that would disrupt every piece 
of logic (kernel modules and userspace code) currently pointing inside 
its memory space.  It would require tearing down the application space 
and unloading every kernel module using the FPGA space (some of which is 
ours and others which are stock kernel code).  Might as well reboot - so 
we load once at boot time.  If the image needs to be swapped out (and 
there are defined cases for this), a symbolic link is updated to point 
to the new bitfile and the processor is rebooted.  But this is getting 
off-topic.

We actually don't directly invoke the use of swnode.  It's being done 
indirectly inside of platform_device_register_full(), when its 
pdevinfo->properties argument is non-null, which is a normal thing that 
happens in many other invocations of that function.  See 
drivers/base/platform.c at the call to 
device_create_managed_software_node().  I had no idea of even the 
existence of swnode until I started tracking down the kernel 
misbehavior.

Whether statically provided or calculated, this code path is happening.  
Our case may be dynamic, but certainly any other managed swnode usage 
which involves child devices is going to be a reference-counting problem 
before this patch.

   [snip]

  -Mike
   isely@pobox.com

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Andy Shevchenko]

On Thu, Feb 26, 2026 at 01:06:42PM -0600, Mike Isely wrote:
> On Thu, 26 Feb 2026, Andy Shevchenko wrote:
> > On Wed, Feb 25, 2026 at 02:16:39PM -0600, Mike Isely wrote:
> > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > > > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:

...

> > > > > > > A scenario exists where device_create_managed_software_node() is used
> > > > > > > to create an swnode instance that will be implicitly shared to a child
> > > > > > > device despite best intentions not to permit such sharing (per the
> > > > > > > comment in device_create_managed_software_node()).  I encountered this
> > > > > > > with the sfp kernel module when it was instantiated with properties
> > > > > > 
> > > > > > SFP? Or is it the name of the actual module in the kernel?
> > > > > 
> > > > > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> > > > > the piece of hardware it works with, an SFP cage.  This is logic which 
> > > > > monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> > > > > When a transceiver appears, the sfp kernel module will create a child 
> > > > > hwmon device instance to monitor various bits of metadata from the 
> > > > > transceiver.  When that transceiver goes away, the sfp kernel module 
> > > > > will tear down that child hwmon device instance.
> > > > > 
> > > > > The sfp kernel module needs resources configured to know where to 
> > > > > monitor; in our case that is set up dynamically by another locally 
> > > > > written kernel module (which iteracts with an FPGA we have where the SFP 
> > > > > hardware elements reside), and that kernel module will combine 
> > > > 
> > > > > devicetree information with some run-time information to generate the 
> > > > > properties handed off to the sfp kernel module instantiation.
> > > > 
> > > > What runtime information? Why this can't be done via DT overlay as others do?
> > > 
> > > I don't recall the specifics.  It might be calculation of a unit name.  
> > > The connectivity in this case is I2C so that should be a constant.  We 
> > > have some variants where the FPGA is PCIE-connected to the host and so 
> > > the memory map is a run-time calculation.  We have other drivers that 
> > > have to be instantiated with run-time computed properties.  So we handle 
> > > this as a general case.
> > 
> > But the configurations are semi-static, right? For the contents of FPGA we have
> > a specific manager that reloads the FPGA configuration.
> > 
> > Using swnode for dynamically calculated data seems weird. The data in swnodes
> > is usually static (const), I can't remember the case where we need to supply
> > run-time calculated values.
> 
> Semi-static means partially dynamic...
> 
> We use the kernel FPGA manager in one platform (zynqmp) and a userspace 
> loader program in another (PCIE connected to bcm2711 soc).  However it 
> is not reasonable to reload on-the-fly as that would disrupt every piece 
> of logic (kernel modules and userspace code) currently pointing inside 
> its memory space.  It would require tearing down the application space 
> and unloading every kernel module using the FPGA space (some of which is 
> ours and others which are stock kernel code).  Might as well reboot - so 
> we load once at boot time.  If the image needs to be swapped out (and 
> there are defined cases for this), a symbolic link is updated to point 
> to the new bitfile and the processor is rebooted.

To me the (full) reversing of the kernel state seems the correct approach.
The modules not really needs to be unloaded. The driver should be unbound
from the device.

> But this is getting off-topic.

Not really, it gets more understanding on why this way was chosen and not
another.

> We actually don't directly invoke the use of swnode.  It's being done 
> indirectly inside of platform_device_register_full(), when its 
> pdevinfo->properties argument is non-null, which is a normal thing that 
> happens in many other invocations of that function.  See 
> drivers/base/platform.c at the call to 
> device_create_managed_software_node().

I'm aware of this. I reviewed a lot of swnode code.

So, the properties supplied are not const, that's what you are trying to say?
And this I believe a problem. I don't remember if we have such a case in kernel
(when we use dynamic properties with platform_device_register_full().

> I had no idea of even the existence of swnode until I started tracking down
> the kernel misbehavior.

> Whether statically provided or calculated, this code path is happening.  
> Our case may be dynamic, but certainly any other managed swnode usage 
> which involves child devices is going to be a reference-counting problem 
> before this patch.

I have a hardware that uses drivers/mfd/intel_quark_i2c_gpio.c. Do you think
I can reproduce the issue with it?

The hierarchy is that

PCI device (associated ACPI node)
  I2C host controller (associated node in ACPI and managed swnode from the driver)
    I2C-client (it has it's own ACPI node)
  GPIO controller (associated node in ACPI and managed swnode from the driver)
    GPIO controller port (child swnode from the driver)
      ...users of GPIO pins of the given port...


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Mike Isely]

On Thu, 26 Feb 2026, Andy Shevchenko wrote:

> On Thu, Feb 26, 2026 at 01:06:42PM -0600, Mike Isely wrote:
> > On Thu, 26 Feb 2026, Andy Shevchenko wrote:
> > > On Wed, Feb 25, 2026 at 02:16:39PM -0600, Mike Isely wrote:
> > > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > > On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > > > > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > > > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:
> 
> ...
> 
> > > > > > > > A scenario exists where device_create_managed_software_node() is used
> > > > > > > > to create an swnode instance that will be implicitly shared to a child
> > > > > > > > device despite best intentions not to permit such sharing (per the
> > > > > > > > comment in device_create_managed_software_node()).  I encountered this
> > > > > > > > with the sfp kernel module when it was instantiated with properties
> > > > > > > 
> > > > > > > SFP? Or is it the name of the actual module in the kernel?
> > > > > > 
> > > > > > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> > > > > > the piece of hardware it works with, an SFP cage.  This is logic which 
> > > > > > monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> > > > > > When a transceiver appears, the sfp kernel module will create a child 
> > > > > > hwmon device instance to monitor various bits of metadata from the 
> > > > > > transceiver.  When that transceiver goes away, the sfp kernel module 
> > > > > > will tear down that child hwmon device instance.
> > > > > > 
> > > > > > The sfp kernel module needs resources configured to know where to 
> > > > > > monitor; in our case that is set up dynamically by another locally 
> > > > > > written kernel module (which iteracts with an FPGA we have where the SFP 
> > > > > > hardware elements reside), and that kernel module will combine 
> > > > > 
> > > > > > devicetree information with some run-time information to generate the 
> > > > > > properties handed off to the sfp kernel module instantiation.
> > > > > 
> > > > > What runtime information? Why this can't be done via DT overlay as others do?
> > > > 
> > > > I don't recall the specifics.  It might be calculation of a unit name.  
> > > > The connectivity in this case is I2C so that should be a constant.  We 
> > > > have some variants where the FPGA is PCIE-connected to the host and so 
> > > > the memory map is a run-time calculation.  We have other drivers that 
> > > > have to be instantiated with run-time computed properties.  So we handle 
> > > > this as a general case.
> > > 
> > > But the configurations are semi-static, right? For the contents of FPGA we have
> > > a specific manager that reloads the FPGA configuration.
> > > 
> > > Using swnode for dynamically calculated data seems weird. The data in swnodes
> > > is usually static (const), I can't remember the case where we need to supply
> > > run-time calculated values.
> > 
> > Semi-static means partially dynamic...
> > 
> > We use the kernel FPGA manager in one platform (zynqmp) and a userspace 
> > loader program in another (PCIE connected to bcm2711 soc).  However it 
> > is not reasonable to reload on-the-fly as that would disrupt every piece 
> > of logic (kernel modules and userspace code) currently pointing inside 
> > its memory space.  It would require tearing down the application space 
> > and unloading every kernel module using the FPGA space (some of which is 
> > ours and others which are stock kernel code).  Might as well reboot - so 
> > we load once at boot time.  If the image needs to be swapped out (and 
> > there are defined cases for this), a symbolic link is updated to point 
> > to the new bitfile and the processor is rebooted.
> 
> To me the (full) reversing of the kernel state seems the correct approach.
> The modules not really needs to be unloaded. The driver should be unbound
> from the device.
> 
> > But this is getting off-topic.
> 
> Not really, it gets more understanding on why this way was chosen and not
> another.

I think there's a miscommunication here.  The nature / mechanism by how 
/ when we load the FPGA has exactly nothing to do with this problem.  
The issue has to do with sfp kernel module behavior when SFP 
transceivers are hotplugged, which is completely unrelated to FPGA 
loading.  The problem would be the same no matter how we manage the 
FPGA.

> 
> > We actually don't directly invoke the use of swnode.  It's being done 
> > indirectly inside of platform_device_register_full(), when its 
> > pdevinfo->properties argument is non-null, which is a normal thing that 
> > happens in many other invocations of that function.  See 
> > drivers/base/platform.c at the call to 
> > device_create_managed_software_node().
> 
> I'm aware of this. I reviewed a lot of swnode code.
> 
> So, the properties supplied are not const, that's what you are trying to say?
> And this I believe a problem. I don't remember if we have such a case in kernel
> (when we use dynamic properties with platform_device_register_full().

In our case it's not const, but I think you're getting hung up on that.  
The const-ness of the data is not germane to the problem.

It isn't that the data is dynamic, it's that swnode is being used at all 
in a scenario where a parent device is managing it and there are child 
devices that come & go over time (due in our case to SFP transceiver 
hotplug).  That scenario happens if platform_device_register_full() is 
called, with the pdevinfo->properties argument set, to create a device 
that itself may later generate child devices that come and go over time.  
The platform_device_register_full() implementation notices that it's 
getting a properties structure via its pdevinfo argument, triggering a 
usage of device_create_managed_software_node().  It doesn't matter if 
the incoming properties to platform_device_register_full() are static or 
not; the behavior will be the same.

We are using platform_device_register_full() to instantiate the sfp 
kernel module.  A grep through the kernel sources shows many other cases 
of this sort of pattern (call to platform_device_register_full() with 
pdevinfo->properties set), so unless every single one of those cases 
happens to not create / remove child devices, this problem is not 
specific to our usage of the sfp kernel module.  And there's certainly 
no rule written anywhere that though shalt not use 
platform_device_register_full() in cases where child devices may be 
created / removed...



> 
> > I had no idea of even the existence of swnode until I started tracking down
> > the kernel misbehavior.
> 
> > Whether statically provided or calculated, this code path is happening.  
> > Our case may be dynamic, but certainly any other managed swnode usage 
> > which involves child devices is going to be a reference-counting problem 
> > before this patch.
> 
> I have a hardware that uses drivers/mfd/intel_quark_i2c_gpio.c. Do you think
> I can reproduce the issue with it?
> 
> The hierarchy is that
> 
> PCI device (associated ACPI node)
>   I2C host controller (associated node in ACPI and managed swnode from the driver)
>     I2C-client (it has it's own ACPI node)
>   GPIO controller (associated node in ACPI and managed swnode from the driver)
>     GPIO controller port (child swnode from the driver)
>       ...users of GPIO pins of the given port...
> 

I am unfamiliar with that driver or its surroundings, so I don't know if 
it is a viable means to reproduce the problem.  I can spend time 
analyzing that code further if you wish.  However it might be more 
productive to try to succinctly, generically, spell out the steps that 
lead to trouble:

1. Somebody creates a struct device instance.

2. Somebody calls device_create_managed_software_node() associating it 
with that struct device instance.  This causes two kobject references to 
be counted against the swnode instance (one for the struct device and 
one because the managed flag was set).  refcount=2

3. Some time later, a child struct device is created and the properties 
of the parent are shared with the child.  This causes another kobject 
reference to be counted against the swnode instance.  refcount=3

4. Some time later, that child struct device instance goes away (perhaps 
due to a hotplug removal).  During the tear-down, 
software_node_notify_remove() notices that the managed field of the 
swnode instance is set, so TWO kobject references are removed instead of 
the single reference created by the previous step.  refcount=1

5. Repeat step 3.  refcount=2

6. Repeat step 4.  refcount=0

7. Now the kobject reference count inside the swnode instance has 
dropped to zero and so the swnode instance is released.  Notice that the 
parent device is still holding a physical reference, now pointing into 
freed memory.  Chaos likely follows.  If lucky, a NULL pointer kernel 
oops is the result.

I confirmed this exact sequence of behavior in the context of the sfp 
kernel module with appropriate printk debug trace added.  While that's 
going to be hard for you to reproduce without sfp hardware, anything 
else which performs those steps should produce similar results.

The patch fixes this by causing step 4 to only do the second reference 
decrement if the device being torn down is the same one that established 
the managing relationship in the first place.  Then everything stays 
balanced.

Notice that this has nothing to do with whether or not the properties 
are const, nor does it have anything to do with FPGA management.  I 
apologize if I caused confusion leading to that.  It's purely the 
combination of a managed swnode instance used for a device that will 
create / remove child devices over time (due perhaps to hotplug 
activity).

And because platform_device_register_full() will implicitly employ a 
managed swnode instance any time it is passed properties in its pdevinfo 
argument, then by implication anyone calling 
platform_device_register_full() in that manner - which happens in 
multiple other places in the kernel - is at risk for the same issue.

  -Mike

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Andy Shevchenko]

On Fri, Feb 27, 2026 at 11:55:51AM -0600, Mike Isely wrote:
> On Thu, 26 Feb 2026, Andy Shevchenko wrote:
> > On Thu, Feb 26, 2026 at 01:06:42PM -0600, Mike Isely wrote:
> > > On Thu, 26 Feb 2026, Andy Shevchenko wrote:
> > > > On Wed, Feb 25, 2026 at 02:16:39PM -0600, Mike Isely wrote:
> > > > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > > > On Wed, Feb 25, 2026 at 01:42:30PM -0600, Mike Isely wrote:
> > > > > > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > > > > > On Tue, Feb 24, 2026 at 01:19:22PM -0600, mike.isely@cobaltdigital.com wrote:

...

> > > > > > > > > A scenario exists where device_create_managed_software_node() is used
> > > > > > > > > to create an swnode instance that will be implicitly shared to a child
> > > > > > > > > device despite best intentions not to permit such sharing (per the
> > > > > > > > > comment in device_create_managed_software_node()).  I encountered this
> > > > > > > > > with the sfp kernel module when it was instantiated with properties
> > > > > > > > 
> > > > > > > > SFP? Or is it the name of the actual module in the kernel?
> > > > > > > 
> > > > > > > Actual kernel module name, sfp.ko, CONFIG_SFP in .config, named after 
> > > > > > > the piece of hardware it works with, an SFP cage.  This is logic which 
> > > > > > > monitors SFP cages for hotplug appearance / removal of SFP transceivers.  
> > > > > > > When a transceiver appears, the sfp kernel module will create a child 
> > > > > > > hwmon device instance to monitor various bits of metadata from the 
> > > > > > > transceiver.  When that transceiver goes away, the sfp kernel module 
> > > > > > > will tear down that child hwmon device instance.
> > > > > > > 
> > > > > > > The sfp kernel module needs resources configured to know where to 
> > > > > > > monitor; in our case that is set up dynamically by another locally 
> > > > > > > written kernel module (which iteracts with an FPGA we have where the SFP 
> > > > > > > hardware elements reside), and that kernel module will combine 
> > > > > > 
> > > > > > > devicetree information with some run-time information to generate the 
> > > > > > > properties handed off to the sfp kernel module instantiation.
> > > > > > 
> > > > > > What runtime information? Why this can't be done via DT overlay as others do?
> > > > > 
> > > > > I don't recall the specifics.  It might be calculation of a unit name.  
> > > > > The connectivity in this case is I2C so that should be a constant.  We 
> > > > > have some variants where the FPGA is PCIE-connected to the host and so 
> > > > > the memory map is a run-time calculation.  We have other drivers that 
> > > > > have to be instantiated with run-time computed properties.  So we handle 
> > > > > this as a general case.
> > > > 
> > > > But the configurations are semi-static, right? For the contents of FPGA we have
> > > > a specific manager that reloads the FPGA configuration.
> > > > 
> > > > Using swnode for dynamically calculated data seems weird. The data in swnodes
> > > > is usually static (const), I can't remember the case where we need to supply
> > > > run-time calculated values.
> > > 
> > > Semi-static means partially dynamic...
> > > 
> > > We use the kernel FPGA manager in one platform (zynqmp) and a userspace 
> > > loader program in another (PCIE connected to bcm2711 soc).  However it 
> > > is not reasonable to reload on-the-fly as that would disrupt every piece 
> > > of logic (kernel modules and userspace code) currently pointing inside 
> > > its memory space.  It would require tearing down the application space 
> > > and unloading every kernel module using the FPGA space (some of which is 
> > > ours and others which are stock kernel code).  Might as well reboot - so 
> > > we load once at boot time.  If the image needs to be swapped out (and 
> > > there are defined cases for this), a symbolic link is updated to point 
> > > to the new bitfile and the processor is rebooted.
> > 
> > To me the (full) reversing of the kernel state seems the correct approach.
> > The modules not really needs to be unloaded. The driver should be unbound
> > from the device.
> > 
> > > But this is getting off-topic.
> > 
> > Not really, it gets more understanding on why this way was chosen and not
> > another.
> 
> I think there's a miscommunication here.  The nature / mechanism by how 
> / when we load the FPGA has exactly nothing to do with this problem.  
> The issue has to do with sfp kernel module behavior when SFP 
> transceivers are hotplugged, which is completely unrelated to FPGA 
> loading.  The problem would be the same no matter how we manage the 
> FPGA.
> 
> > > We actually don't directly invoke the use of swnode.  It's being done 
> > > indirectly inside of platform_device_register_full(), when its 
> > > pdevinfo->properties argument is non-null, which is a normal thing that 
> > > happens in many other invocations of that function.  See 
> > > drivers/base/platform.c at the call to 
> > > device_create_managed_software_node().
> > 
> > I'm aware of this. I reviewed a lot of swnode code.
> > 
> > So, the properties supplied are not const, that's what you are trying to say?
> > And this I believe a problem. I don't remember if we have such a case in kernel
> > (when we use dynamic properties with platform_device_register_full().
> 
> In our case it's not const, but I think you're getting hung up on that.  
> The const-ness of the data is not germane to the problem.
> 
> It isn't that the data is dynamic, it's that swnode is being used at all 
> in a scenario where a parent device is managing it and there are child 
> devices that come & go over time (due in our case to SFP transceiver 
> hotplug).  That scenario happens if platform_device_register_full() is 
> called, with the pdevinfo->properties argument set, to create a device 
> that itself may later generate child devices that come and go over time.  
> The platform_device_register_full() implementation notices that it's 
> getting a properties structure via its pdevinfo argument, triggering a 
> usage of device_create_managed_software_node().  It doesn't matter if 
> the incoming properties to platform_device_register_full() are static or 
> not; the behavior will be the same.
> 
> We are using platform_device_register_full() to instantiate the sfp 
> kernel module.  A grep through the kernel sources shows many other cases 
> of this sort of pattern (call to platform_device_register_full() with 
> pdevinfo->properties set), so unless every single one of those cases 
> happens to not create / remove child devices, this problem is not 
> specific to our usage of the sfp kernel module.  And there's certainly 
> no rule written anywhere that though shalt not use 
> platform_device_register_full() in cases where child devices may be 
> created / removed...
> 
> > > I had no idea of even the existence of swnode until I started tracking down
> > > the kernel misbehavior.
> > 
> > > Whether statically provided or calculated, this code path is happening.  
> > > Our case may be dynamic, but certainly any other managed swnode usage 
> > > which involves child devices is going to be a reference-counting problem 
> > > before this patch.
> > 
> > I have a hardware that uses drivers/mfd/intel_quark_i2c_gpio.c. Do you think
> > I can reproduce the issue with it?
> > 
> > The hierarchy is that
> > 
> > PCI device (associated ACPI node)
> >   I2C host controller (associated node in ACPI and managed swnode from the driver)
> >     I2C-client (it has it's own ACPI node)
> >   GPIO controller (associated node in ACPI and managed swnode from the driver)
> >     GPIO controller port (child swnode from the driver)
> >       ...users of GPIO pins of the given port...
> 
> I am unfamiliar with that driver or its surroundings, so I don't know if 
> it is a viable means to reproduce the problem.  I can spend time 
> analyzing that code further if you wish.  However it might be more 
> productive to try to succinctly, generically, spell out the steps that 
> lead to trouble:
> 
> 1. Somebody creates a struct device instance.
> 
> 2. Somebody calls device_create_managed_software_node() associating it 
> with that struct device instance.  This causes two kobject references to 
> be counted against the swnode instance (one for the struct device and 
> one because the managed flag was set).  refcount=2
> 
> 3. Some time later, a child struct device is created and the properties 
> of the parent are shared with the child.  This causes another kobject 
> reference to be counted against the swnode instance.  refcount=3
> 
> 4. Some time later, that child struct device instance goes away (perhaps 
> due to a hotplug removal).  During the tear-down, 
> software_node_notify_remove() notices that the managed field of the 
> swnode instance is set, so TWO kobject references are removed instead of 
> the single reference created by the previous step.  refcount=1
> 
> 5. Repeat step 3.  refcount=2
> 
> 6. Repeat step 4.  refcount=0
> 
> 7. Now the kobject reference count inside the swnode instance has 
> dropped to zero and so the swnode instance is released.  Notice that the 
> parent device is still holding a physical reference, now pointing into 
> freed memory.  Chaos likely follows.  If lucky, a NULL pointer kernel 
> oops is the result.
> 
> I confirmed this exact sequence of behavior in the context of the sfp 
> kernel module with appropriate printk debug trace added.  While that's 
> going to be hard for you to reproduce without sfp hardware, anything 
> else which performs those steps should produce similar results.
> 
> The patch fixes this by causing step 4 to only do the second reference 
> decrement if the device being torn down is the same one that established 
> the managing relationship in the first place.  Then everything stays 
> balanced.
> 
> Notice that this has nothing to do with whether or not the properties 
> are const, nor does it have anything to do with FPGA management.  I 
> apologize if I caused confusion leading to that.

No problem, now with this very deep and comprehensive analysis everything
is clear. Thanks for providing this all information!

> It's purely the
> combination of a managed swnode instance used for a device that will 
> create / remove child devices over time (due perhaps to hotplug 
> activity).

> And because platform_device_register_full() will implicitly employ a 
> managed swnode instance any time it is passed properties in its pdevinfo 
> argument, then by implication anyone calling 
> platform_device_register_full() in that manner - which happens in 
> multiple other places in the kernel - is at risk for the same issue.

Yeah, I will think about this more, but if somebody beats me up to it
and confirms that this is the best what we can do, I will have no
objections. I dunno, let's say in a couple of weeks?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node

[Mike Isely]

On Sat, 28 Feb 2026, Andy Shevchenko wrote:

> On Fri, Feb 27, 2026 at 11:55:51AM -0600, Mike Isely wrote:
> > On Thu, 26 Feb 2026, Andy Shevchenko wrote:

  [snip]

> 
> > I am unfamiliar with that driver or its surroundings, so I don't know if 
> > it is a viable means to reproduce the problem.  I can spend time 
> > analyzing that code further if you wish.  However it might be more 
> > productive to try to succinctly, generically, spell out the steps that 
> > lead to trouble:
> > 
> > 1. Somebody creates a struct device instance.
> > 
> > 2. Somebody calls device_create_managed_software_node() associating it 
> > with that struct device instance.  This causes two kobject references to 
> > be counted against the swnode instance (one for the struct device and 
> > one because the managed flag was set).  refcount=2
> > 
> > 3. Some time later, a child struct device is created and the properties 
> > of the parent are shared with the child.  This causes another kobject 
> > reference to be counted against the swnode instance.  refcount=3
> > 
> > 4. Some time later, that child struct device instance goes away (perhaps 
> > due to a hotplug removal).  During the tear-down, 
> > software_node_notify_remove() notices that the managed field of the 
> > swnode instance is set, so TWO kobject references are removed instead of 
> > the single reference created by the previous step.  refcount=1
> > 
> > 5. Repeat step 3.  refcount=2
> > 
> > 6. Repeat step 4.  refcount=0
> > 
> > 7. Now the kobject reference count inside the swnode instance has 
> > dropped to zero and so the swnode instance is released.  Notice that the 
> > parent device is still holding a physical reference, now pointing into 
> > freed memory.  Chaos likely follows.  If lucky, a NULL pointer kernel 
> > oops is the result.
> > 
> > I confirmed this exact sequence of behavior in the context of the sfp 
> > kernel module with appropriate printk debug trace added.  While that's 
> > going to be hard for you to reproduce without sfp hardware, anything 
> > else which performs those steps should produce similar results.
> > 
> > The patch fixes this by causing step 4 to only do the second reference 
> > decrement if the device being torn down is the same one that established 
> > the managing relationship in the first place.  Then everything stays 
> > balanced.
> > 
> > Notice that this has nothing to do with whether or not the properties 
> > are const, nor does it have anything to do with FPGA management.  I 
> > apologize if I caused confusion leading to that.
> 
> No problem, now with this very deep and comprehensive analysis everything
> is clear. Thanks for providing this all information!
> 
> > It's purely the
> > combination of a managed swnode instance used for a device that will 
> > create / remove child devices over time (due perhaps to hotplug 
> > activity).
> 
> > And because platform_device_register_full() will implicitly employ a 
> > managed swnode instance any time it is passed properties in its pdevinfo 
> > argument, then by implication anyone calling 
> > platform_device_register_full() in that manner - which happens in 
> > multiple other places in the kernel - is at risk for the same issue.
> 
> Yeah, I will think about this more, but if somebody beats me up to it
> and confirms that this is the best what we can do, I will have no
> objections. I dunno, let's say in a couple of weeks?
> 
> 

No rush.  We're already using the patch internally of course.

I can resend you the patch, fixing the process issues you cited, along 
with the commit comment adjusted to include the sequence above which 
finally made the issue crystal clear.  I'll try to get that to you on 
Monday or Tuesday.

  -Mike
   isely@pobox.com

Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 01:19:21PM -0600, mike.isely@cobaltdigital.com wrote:

> Correct issue in drivers/base/swnode.c that can lead to use-after-free
> due to kobject reference counting error, which itself is due to
> incorrect behavior with the "managed" struct swnode flag in
> circumstances involving child struct device instances where the parent
> struct device is managing a struct swnode.
> 
> Use-after-free in this case led to an Oops and a subsequent kernel
> memory leak, but realistically it's kernel heap corruption, so any
> manner of chaos can result, if left unaddressed.
> 
> This was detected in kernel 6.12, verified also in kernel 6.6.  Visual
> inspection in 6.19.3 source (the latest as of right now) shows the

The latest is v7.0-rc1 as of time of the topic message.

> same issue.  The nearly trivial fix was verified in 6.12.  While this
> patches against 6.19.3, IMHO this is a candidate for all LTS kernels.

Thanks for the contribution, usually for a single patch there is no need
in cover letter. The comment block can handle this (the place after cutter
'---' line in the message with a patch).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[Mike Isely]

On Wed, 25 Feb 2026, Andy Shevchenko wrote:

> On Tue, Feb 24, 2026 at 01:19:21PM -0600, mike.isely@cobaltdigital.com wrote:
> 
> > Correct issue in drivers/base/swnode.c that can lead to use-after-free
> > due to kobject reference counting error, which itself is due to
> > incorrect behavior with the "managed" struct swnode flag in
> > circumstances involving child struct device instances where the parent
> > struct device is managing a struct swnode.
> > 
> > Use-after-free in this case led to an Oops and a subsequent kernel
> > memory leak, but realistically it's kernel heap corruption, so any
> > manner of chaos can result, if left unaddressed.
> > 
> > This was detected in kernel 6.12, verified also in kernel 6.6.  Visual
> > inspection in 6.19.3 source (the latest as of right now) shows the
> 
> The latest is v7.0-rc1 as of time of the topic message.

I actually meant the latest release.  Guess I should have checked the 
latest release candidate on the off-chance that it might have been 
addressed.

> 
> > same issue.  The nearly trivial fix was verified in 6.12.  While this
> > patches against 6.19.3, IMHO this is a candidate for all LTS kernels.
> 
> Thanks for the contribution, usually for a single patch there is no need
> in cover letter. The comment block can handle this (the place after cutter
> '---' line in the message with a patch).

Yeah, a separate cover letter is overkill, but I was just following a 
process here.

  -Mike
   isely@pobox.com

Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 12:59:56PM -0600, Mike Isely wrote:
> On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > On Tue, Feb 24, 2026 at 01:19:21PM -0600, mike.isely@cobaltdigital.com wrote:

...

> > > This was detected in kernel 6.12, verified also in kernel 6.6.  Visual
> > > inspection in 6.19.3 source (the latest as of right now) shows the
> > 
> > The latest is v7.0-rc1 as of time of the topic message.
> 
> I actually meant the latest release.  Guess I should have checked the 
> latest release candidate on the off-chance that it might have been 
> addressed.

It is probably not, but the idea to check against latest tag in the vanilla
repository. v6.19.3 is not even vanilla, it's stable kernel.

> > > same issue.  The nearly trivial fix was verified in 6.12.  While this
> > > patches against 6.19.3, IMHO this is a candidate for all LTS kernels.
> > 
> > Thanks for the contribution, usually for a single patch there is no need
> > in cover letter. The comment block can handle this (the place after cutter
> > '---' line in the message with a patch).
> 
> Yeah, a separate cover letter is overkill, but I was just following a 
> process here.

What process? I think we have that somewhere in the documentation that cover
letter for a single patch is not needed...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[Mike Isely]

On Wed, 25 Feb 2026, Andy Shevchenko wrote:

> On Wed, Feb 25, 2026 at 12:59:56PM -0600, Mike Isely wrote:
> > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > On Tue, Feb 24, 2026 at 01:19:21PM -0600, mike.isely@cobaltdigital.com wrote:
> 
> ...
> 
> > > > This was detected in kernel 6.12, verified also in kernel 6.6.  Visual
> > > > inspection in 6.19.3 source (the latest as of right now) shows the
> > > 
> > > The latest is v7.0-rc1 as of time of the topic message.
> > 
> > I actually meant the latest release.  Guess I should have checked the 
> > latest release candidate on the off-chance that it might have been 
> > addressed.
> 
> It is probably not, but the idea to check against latest tag in the vanilla
> repository. v6.19.3 is not even vanilla, it's stable kernel.

I tend to stick with the latest kernel that is NOT a release candidate 
when building random things here regardless of the term used and that's 
still 6.19.3.  But for verifying a patch, yes I should have at least 
taken a closer look at 7.0-rc1.


> 
> > > > same issue.  The nearly trivial fix was verified in 6.12.  While this
> > > > patches against 6.19.3, IMHO this is a candidate for all LTS kernels.
> > > 
> > > Thanks for the contribution, usually for a single patch there is no need
> > > in cover letter. The comment block can handle this (the place after cutter
> > > '---' line in the message with a patch).
> > 
> > Yeah, a separate cover letter is overkill, but I was just following a 
> > process here.
> 
> What process? I think we have that somewhere in the documentation that cover
> letter for a single patch is not needed...

Documentation/process/submitting-patches.rst in the kernel sources, or
https://docs.kernel.org/process/submitting-patches.html

  -Mike
   isely@pobox.com

Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 01:48:04PM -0600, Mike Isely wrote:
> On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > On Wed, Feb 25, 2026 at 12:59:56PM -0600, Mike Isely wrote:
> > > On Wed, 25 Feb 2026, Andy Shevchenko wrote:
> > > > On Tue, Feb 24, 2026 at 01:19:21PM -0600, mike.isely@cobaltdigital.com wrote:

...

> > > > > This was detected in kernel 6.12, verified also in kernel 6.6.  Visual
> > > > > inspection in 6.19.3 source (the latest as of right now) shows the
> > > > 
> > > > The latest is v7.0-rc1 as of time of the topic message.
> > > 
> > > I actually meant the latest release.  Guess I should have checked the 
> > > latest release candidate on the off-chance that it might have been 
> > > addressed.
> > 
> > It is probably not, but the idea to check against latest tag in the vanilla
> > repository. v6.19.3 is not even vanilla, it's stable kernel.
> 
> I tend to stick with the latest kernel that is NOT a release candidate 
> when building random things here regardless of the term used and that's 
> still 6.19.3.  But for verifying a patch, yes I should have at least 
> taken a closer look at 7.0-rc1.

The logical requirement for a new contribution is to build changes against
current or next cycle. Hence only two kernels have interest to us:

- latest tag in vanilla (v7.0-rc1 as of today)
- latest tag in Linux Next (whatever day it is)

> > > > > same issue.  The nearly trivial fix was verified in 6.12.  While this
> > > > > patches against 6.19.3, IMHO this is a candidate for all LTS kernels.
> > > > 
> > > > Thanks for the contribution, usually for a single patch there is no need
> > > > in cover letter. The comment block can handle this (the place after cutter
> > > > '---' line in the message with a patch).
> > > 
> > > Yeah, a separate cover letter is overkill, but I was just following a 
> > > process here.
> > 
> > What process? I think we have that somewhere in the documentation that cover
> > letter for a single patch is not needed...
> 
> Documentation/process/submitting-patches.rst in the kernel sources, or
> https://docs.kernel.org/process/submitting-patches.html

Yes, it maybe not so clear but it actually tells you to choose between two:
- vanilla (latest tag), OR
- dedicated tree "for-next" (most use this, but some use other name for
  the branch for the next cycle).


-- 
With Best Regards,
Andy Shevchenko