* [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c
@ 2026-02-24 19:19 mike.isely
2026-02-24 19:19 ` [PATCH 1/1] sofware node: Only the managing device can unreference managed software node mike.isely
2026-02-25 9:46 ` [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c Andy Shevchenko
0 siblings, 2 replies; 17+ messages in thread
From: mike.isely @ 2026-02-24 19:19 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus
Cc: Mike Isely, Mike Isely, linux-acpi, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-24 19:19 [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c mike.isely
@ 2026-02-24 19:19 ` mike.isely
2026-02-25 11:22 ` Andy Shevchenko
2026-02-25 9:46 ` [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c Andy Shevchenko
1 sibling, 1 reply; 17+ messages in thread
From: mike.isely @ 2026-02-24 19:19 UTC (permalink / raw)
To: Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus
Cc: Mike Isely, Mike Isely, linux-acpi, linux-kernel
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c
2026-02-24 19:19 [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c mike.isely
2026-02-24 19:19 ` [PATCH 1/1] sofware node: Only the managing device can unreference managed software node mike.isely
@ 2026-02-25 9:46 ` Andy Shevchenko
2026-02-25 18:59 ` Mike Isely
1 sibling, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-25 9:46 UTC (permalink / raw)
To: mike.isely
Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Mike Isely,
linux-acpi, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-24 19:19 ` [PATCH 1/1] sofware node: Only the managing device can unreference managed software node mike.isely
@ 2026-02-25 11:22 ` Andy Shevchenko
2026-02-25 19:42 ` Mike Isely
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-25 11:22 UTC (permalink / raw)
To: mike.isely
Cc: Daniel Scally, Heikki Krogerus, Sakari Ailus, Mike Isely,
linux-acpi, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c
2026-02-25 9:46 ` [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c Andy Shevchenko
@ 2026-02-25 18:59 ` Mike Isely
2026-02-25 19:17 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Mike Isely @ 2026-02-25 18:59 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c
2026-02-25 18:59 ` Mike Isely
@ 2026-02-25 19:17 ` Andy Shevchenko
2026-02-25 19:48 ` Mike Isely
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-25 19:17 UTC (permalink / raw)
To: Mike Isely at pobox
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-25 11:22 ` Andy Shevchenko
@ 2026-02-25 19:42 ` Mike Isely
2026-02-25 20:01 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Mike Isely @ 2026-02-25 19:42 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c
2026-02-25 19:17 ` Andy Shevchenko
@ 2026-02-25 19:48 ` Mike Isely
2026-02-25 20:05 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Mike Isely @ 2026-02-25 19:48 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-25 19:42 ` Mike Isely
@ 2026-02-25 20:01 ` Andy Shevchenko
2026-02-25 20:16 ` Mike Isely
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-25 20:01 UTC (permalink / raw)
To: Mike Isely at pobox
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c
2026-02-25 19:48 ` Mike Isely
@ 2026-02-25 20:05 ` Andy Shevchenko
0 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-25 20:05 UTC (permalink / raw)
To: Mike Isely at pobox
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-25 20:01 ` Andy Shevchenko
@ 2026-02-25 20:16 ` Mike Isely
2026-02-26 7:16 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Mike Isely @ 2026-02-25 20:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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.
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-25 20:16 ` Mike Isely
@ 2026-02-26 7:16 ` Andy Shevchenko
2026-02-26 19:06 ` Mike Isely
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-26 7:16 UTC (permalink / raw)
To: Mike Isely at pobox
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-26 7:16 ` Andy Shevchenko
@ 2026-02-26 19:06 ` Mike Isely
2026-02-26 20:42 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Mike Isely @ 2026-02-26 19:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-26 19:06 ` Mike Isely
@ 2026-02-26 20:42 ` Andy Shevchenko
2026-02-27 17:55 ` Mike Isely
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-26 20:42 UTC (permalink / raw)
To: Mike Isely at pobox
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-26 20:42 ` Andy Shevchenko
@ 2026-02-27 17:55 ` Mike Isely
2026-02-28 11:02 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Mike Isely @ 2026-02-27 17:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-27 17:55 ` Mike Isely
@ 2026-02-28 11:02 ` Andy Shevchenko
2026-02-28 16:34 ` Mike Isely
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2026-02-28 11:02 UTC (permalink / raw)
To: Mike Isely at pobox
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/1] sofware node: Only the managing device can unreference managed software node
2026-02-28 11:02 ` Andy Shevchenko
@ 2026-02-28 16:34 ` Mike Isely
0 siblings, 0 replies; 17+ messages in thread
From: Mike Isely @ 2026-02-28 16:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: mike.isely, Daniel Scally, Heikki Krogerus, Sakari Ailus,
linux-acpi, Linux Kernel Mailing List, Mike Isely at pobox
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-02-28 16:34 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 19:19 [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c mike.isely
2026-02-24 19:19 ` [PATCH 1/1] sofware node: Only the managing device can unreference managed software node mike.isely
2026-02-25 11:22 ` Andy Shevchenko
2026-02-25 19:42 ` Mike Isely
2026-02-25 20:01 ` Andy Shevchenko
2026-02-25 20:16 ` Mike Isely
2026-02-26 7:16 ` Andy Shevchenko
2026-02-26 19:06 ` Mike Isely
2026-02-26 20:42 ` Andy Shevchenko
2026-02-27 17:55 ` Mike Isely
2026-02-28 11:02 ` Andy Shevchenko
2026-02-28 16:34 ` Mike Isely
2026-02-25 9:46 ` [PATCH 0/1] software node: Use-after-free fix in drivers/base/swnode.c Andy Shevchenko
2026-02-25 18:59 ` Mike Isely
2026-02-25 19:17 ` Andy Shevchenko
2026-02-25 19:48 ` Mike Isely
2026-02-25 20:05 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox