[PATCH] software node: only let the owner release a managed node

[PATCH] software node: only let the owner release a managed node

[Bartosz Golaszewski]

device_create_managed_software_node() creates a node whose lifetime is
tied to the device it was created for. software_node_notify_remove() is
called for every device that has the node as its fwnode (primary OR
secondary) and, if the node is managed, unconditionally detaches it from
that device and drops the creation reference.

This breaks when the node is shared with another device. Example: the
i2c-designware copies the parent's fwnode to the adapter's struct device.
When the adapter is deleted, it releases the reference that belongs to
the owner and the latter ends up with a dangling pointer.

Record the device's "manager" and only perform the actual release when
notify is called for that "owner" device.

Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
Fixes: 151f6ff78cdf ("software node: Provide replacement for device_add_properties()")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Hi!

This is my approach at addressing the bug reported by Vivekanandan
directly in driver core. Please test it if you can.

Thanks,
Bartosz
---
 drivers/base/swnode.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 84a77ffa3dc4bbcca62bbe67c3390693500b01ea..abb6d6d59d59cb4e605fc31cc7e89f92e9f900a6 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -29,6 +29,8 @@ struct swnode {
 	struct fwnode_handle fwnode;
 	const struct software_node *node;
 	int id;
+	/* Device the managed node's lifetime is tied to. */
+	struct device *owner;
 
 	/* hierarchy */
 	struct ida child_ids;
@@ -37,7 +39,6 @@ struct swnode {
 	struct swnode *parent;
 
 	unsigned int allocated:1;
-	unsigned int managed:1;
 };
 
 static DEFINE_IDA(swnode_root_ids);
@@ -1145,7 +1146,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)->owner = dev;
 	set_secondary_fwnode(dev, fwnode);
 
 	if (device_is_registered(dev))
@@ -1190,6 +1191,7 @@ void software_node_notify(struct device *dev)
 void software_node_notify_remove(struct device *dev)
 {
 	struct swnode *swnode;
+	bool is_owner;
 
 	swnode = dev_to_swnode(dev);
 	if (!swnode)
@@ -1206,9 +1208,13 @@ void software_node_notify_remove(struct device *dev)
 	if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
 		swnode->fwnode.dev = NULL;
 
+	is_owner = swnode->owner && swnode->owner == dev;
+	if (is_owner)
+		swnode->owner = NULL;
+
 	swnode_put(swnode);
 
-	if (swnode->managed) {
+	if (is_owner) {
 		set_secondary_fwnode(dev, NULL);
 		swnode_put(swnode);
 	}

---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
change-id: 20261102-swnode-only-owner-release-45efdc7123e1

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH] software node: only let the owner release a managed node

[Vivekanandan, Balasubramani]

On 02.11.2026 16:25, Bartosz Golaszewski wrote:
> device_create_managed_software_node() creates a node whose lifetime is
> tied to the device it was created for. software_node_notify_remove() is
> called for every device that has the node as its fwnode (primary OR
> secondary) and, if the node is managed, unconditionally detaches it from
> that device and drops the creation reference.
> 
> This breaks when the node is shared with another device. Example: the
> i2c-designware copies the parent's fwnode to the adapter's struct device.
> When the adapter is deleted, it releases the reference that belongs to
> the owner and the latter ends up with a dangling pointer.
> 
> Record the device's "manager" and only perform the actual release when
> notify is called for that "owner" device.
> 
> Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
> Fixes: 151f6ff78cdf ("software node: Provide replacement for device_add_properties()")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> Hi!
> 
> This is my approach at addressing the bug reported by Vivekanandan
> directly in driver core. Please test it if you can.
> 
> Thanks,
> Bartosz
> ---
>  drivers/base/swnode.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Do you want to update the comment above
device_create_managed_software_node() which says reusing or sharing of
software nodes created by the function is restricted?

Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

Regards,
Bala

> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 84a77ffa3dc4bbcca62bbe67c3390693500b01ea..abb6d6d59d59cb4e605fc31cc7e89f92e9f900a6 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -29,6 +29,8 @@ struct swnode {
>  	struct fwnode_handle fwnode;
>  	const struct software_node *node;
>  	int id;
> +	/* Device the managed node's lifetime is tied to. */
> +	struct device *owner;
>  
>  	/* hierarchy */
>  	struct ida child_ids;
> @@ -37,7 +39,6 @@ struct swnode {
>  	struct swnode *parent;
>  
>  	unsigned int allocated:1;
> -	unsigned int managed:1;
>  };
>  
>  static DEFINE_IDA(swnode_root_ids);
> @@ -1145,7 +1146,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)->owner = dev;
>  	set_secondary_fwnode(dev, fwnode);
>  
>  	if (device_is_registered(dev))
> @@ -1190,6 +1191,7 @@ void software_node_notify(struct device *dev)
>  void software_node_notify_remove(struct device *dev)
>  {
>  	struct swnode *swnode;
> +	bool is_owner;
>  
>  	swnode = dev_to_swnode(dev);
>  	if (!swnode)
> @@ -1206,9 +1208,13 @@ void software_node_notify_remove(struct device *dev)
>  	if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
>  		swnode->fwnode.dev = NULL;
>  
> +	is_owner = swnode->owner && swnode->owner == dev;
> +	if (is_owner)
> +		swnode->owner = NULL;
> +
>  	swnode_put(swnode);
>  
> -	if (swnode->managed) {
> +	if (is_owner) {
>  		set_secondary_fwnode(dev, NULL);
>  		swnode_put(swnode);
>  	}
> 
> ---
> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
> change-id: 20261102-swnode-only-owner-release-45efdc7123e1
> 
> Best regards,
> -- 
> Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> 

Re: [PATCH] software node: only let the owner release a managed node

[Andy Shevchenko]

On Wed, Nov 04, 2026 at 12:17:01PM +0530, Vivekanandan, Balasubramani wrote:
> On 02.11.2026 16:25, Bartosz Golaszewski wrote:

...

> > Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/

> > ---
> > This is my approach at addressing the bug reported by Vivekanandan
> > directly in driver core. Please test it if you can.
> > ---

> Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

Thanks, would it be possible to test this?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] software node: only let the owner release a managed node

[Vivekanandan, Balasubramani]

On 04.11.2026 10:19, Andy Shevchenko wrote:
> On Wed, Nov 04, 2026 at 12:17:01PM +0530, Vivekanandan, Balasubramani wrote:
> > On 02.11.2026 16:25, Bartosz Golaszewski wrote:
> 
> ...
> 
> > > Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
> 
> > > ---
> > > This is my approach at addressing the bug reported by Vivekanandan
> > > directly in driver core. Please test it if you can.
> > > ---
> 
> > Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> 
> Thanks, would it be possible to test this?

Tested it already. Kernel crash is fixed.

Regards,
Bala

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH] software node: only let the owner release a managed node

[Andy Shevchenko]

On Wed, Nov 04, 2026 at 01:56:50PM +0530, Vivekanandan, Balasubramani wrote:
> On 04.11.2026 10:19, Andy Shevchenko wrote:
> > On Wed, Nov 04, 2026 at 12:17:01PM +0530, Vivekanandan, Balasubramani wrote:
> > > On 02.11.2026 16:25, Bartosz Golaszewski wrote:

...

> > > > Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > > Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
> > 
> > > > ---
> > > > This is my approach at addressing the bug reported by Vivekanandan
> > > > directly in driver core. Please test it if you can.
> > > > ---
> > 
> > > Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > 
> > Thanks, would it be possible to test this?
> 
> Tested it already. Kernel crash is fixed.

Thanks, but we expect a formal Tested-by tag. Reviewed-by is only about how
we read the code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] software node: only let the owner release a managed node

[Vivekanandan, Balasubramani]

On 04.11.2026 10:35, Andy Shevchenko wrote:
> On Wed, Nov 04, 2026 at 01:56:50PM +0530, Vivekanandan, Balasubramani wrote:
> > On 04.11.2026 10:19, Andy Shevchenko wrote:
> > > On Wed, Nov 04, 2026 at 12:17:01PM +0530, Vivekanandan, Balasubramani wrote:
> > > > On 02.11.2026 16:25, Bartosz Golaszewski wrote:
> 
> ...
> 
> > > > > Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > > > Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
> > > 
> > > > > ---
> > > > > This is my approach at addressing the bug reported by Vivekanandan
> > > > > directly in driver core. Please test it if you can.
> > > > > ---
> > > 
> > > > Reviewed-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > 
> > > Thanks, would it be possible to test this?
> > 
> > Tested it already. Kernel crash is fixed.
> 
> Thanks, but we expect a formal Tested-by tag. Reviewed-by is only about how
> we read the code.

Sure.

Tested-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

Regards,
Bala

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH] software node: only let the owner release a managed node

[Heikki Krogerus]

On Mon, Nov 02, 2026 at 04:25:16PM +0100, Bartosz Golaszewski wrote:
> device_create_managed_software_node() creates a node whose lifetime is
> tied to the device it was created for. software_node_notify_remove() is
> called for every device that has the node as its fwnode (primary OR
> secondary) and, if the node is managed, unconditionally detaches it from
> that device and drops the creation reference.
> 
> This breaks when the node is shared with another device. Example: the
> i2c-designware copies the parent's fwnode to the adapter's struct device.
> When the adapter is deleted, it releases the reference that belongs to
> the owner and the latter ends up with a dangling pointer.
> 
> Record the device's "manager" and only perform the actual release when
> notify is called for that "owner" device.
> 
> Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
> Fixes: 151f6ff78cdf ("software node: Provide replacement for device_add_properties()")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

LGTM. FWIW:

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Hi!
> 
> This is my approach at addressing the bug reported by Vivekanandan
> directly in driver core. Please test it if you can.
> 
> Thanks,
> Bartosz
> ---
>  drivers/base/swnode.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 84a77ffa3dc4bbcca62bbe67c3390693500b01ea..abb6d6d59d59cb4e605fc31cc7e89f92e9f900a6 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -29,6 +29,8 @@ struct swnode {
>  	struct fwnode_handle fwnode;
>  	const struct software_node *node;
>  	int id;
> +	/* Device the managed node's lifetime is tied to. */
> +	struct device *owner;
>  
>  	/* hierarchy */
>  	struct ida child_ids;
> @@ -37,7 +39,6 @@ struct swnode {
>  	struct swnode *parent;
>  
>  	unsigned int allocated:1;
> -	unsigned int managed:1;
>  };
>  
>  static DEFINE_IDA(swnode_root_ids);
> @@ -1145,7 +1146,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)->owner = dev;
>  	set_secondary_fwnode(dev, fwnode);
>  
>  	if (device_is_registered(dev))
> @@ -1190,6 +1191,7 @@ void software_node_notify(struct device *dev)
>  void software_node_notify_remove(struct device *dev)
>  {
>  	struct swnode *swnode;
> +	bool is_owner;
>  
>  	swnode = dev_to_swnode(dev);
>  	if (!swnode)
> @@ -1206,9 +1208,13 @@ void software_node_notify_remove(struct device *dev)
>  	if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
>  		swnode->fwnode.dev = NULL;
>  
> +	is_owner = swnode->owner && swnode->owner == dev;
> +	if (is_owner)
> +		swnode->owner = NULL;
> +
>  	swnode_put(swnode);
>  
> -	if (swnode->managed) {
> +	if (is_owner) {
>  		set_secondary_fwnode(dev, NULL);
>  		swnode_put(swnode);
>  	}
> 
> ---
> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
> change-id: 20261102-swnode-only-owner-release-45efdc7123e1
> 
> Best regards,
> -- 
> Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

-- 
heikki

Re: [PATCH] software node: only let the owner release a managed node

[Johan Hovold]

On Mon, Nov 02, 2026 at 04:25:16PM +0100, Bartosz Golaszewski wrote:
> device_create_managed_software_node() creates a node whose lifetime is
> tied to the device it was created for. software_node_notify_remove() is
> called for every device that has the node as its fwnode (primary OR
> secondary) and, if the node is managed, unconditionally detaches it from
> that device and drops the creation reference.
> 
> This breaks when the node is shared with another device. Example: the
> i2c-designware copies the parent's fwnode to the adapter's struct device.
> When the adapter is deleted, it releases the reference that belongs to
> the owner and the latter ends up with a dangling pointer.
> 
> Record the device's "manager" and only perform the actual release when
> notify is called for that "owner" device.
> 
> Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
> Fixes: 151f6ff78cdf ("software node: Provide replacement for device_add_properties()")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> Hi!
> 
> This is my approach at addressing the bug reported by Vivekanandan
> directly in driver core. Please test it if you can.

This looks good to me, but I think the patch should be simplified somewhat.

> @@ -1190,6 +1191,7 @@ void software_node_notify(struct device *dev)
>  void software_node_notify_remove(struct device *dev)
>  {
>  	struct swnode *swnode;
> +	bool is_owner;
>  
>  	swnode = dev_to_swnode(dev);
>  	if (!swnode)
> @@ -1206,9 +1208,13 @@ void software_node_notify_remove(struct device *dev)
>  	if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
>  		swnode->fwnode.dev = NULL;
>  
> +	is_owner = swnode->owner && swnode->owner == dev;

The NULL check here is redundant.

> +	if (is_owner)
> +		swnode->owner = NULL;

There is no need to clear the owner field as devices are not allowed to be
re-registered.

> +
>  	swnode_put(swnode);
>  
> -	if (swnode->managed) {
> +	if (is_owner) {
>  		set_secondary_fwnode(dev, NULL);
>  		swnode_put(swnode);
>  	}

And if we drop the managed reference before the notify one, we can drop
the temporary as well and this chunk just becomes:

@@ -1206,12 +1207,12 @@ void software_node_notify_remove(struct device *dev)
        if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
                swnode->fwnode.dev = NULL;
 
-       swnode_put(swnode);
-
-       if (swnode->managed) {
+       if (swnode->owner == dev) {
                set_secondary_fwnode(dev, NULL);
                swnode_put(swnode);
        }
+
+       swnode_put(swnode);
 }

The end-result applied for 7.4 (to fix the 7.3 regression) is here:
	
	https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/?h=driver-core-linus&id=95418e71bdcbf8f2073a730365c11d22ff8473f3

Johan

Re: [PATCH] software node: only let the owner release a managed node

[Danilo Krummrich]

On Mon Nov 9, 2026 at 2:37 PM CET, Johan Hovold wrote:
> On Mon, Nov 02, 2026 at 04:25:16PM +0100, Bartosz Golaszewski wrote:
>> device_create_managed_software_node() creates a node whose lifetime is
>> tied to the device it was created for. software_node_notify_remove() is
>> called for every device that has the node as its fwnode (primary OR
>> secondary) and, if the node is managed, unconditionally detaches it from
>> that device and drops the creation reference.
>> 
>> This breaks when the node is shared with another device. Example: the
>> i2c-designware copies the parent's fwnode to the adapter's struct device.
>> When the adapter is deleted, it releases the reference that belongs to
>> the owner and the latter ends up with a dangling pointer.
>> 
>> Record the device's "manager" and only perform the actual release when
>> notify is called for that "owner" device.
>> 
>> Reported-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>> Closes: https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/
>> Fixes: 151f6ff78cdf ("software node: Provide replacement for device_add_properties()")

Hm...the documentation of device_create_managed_software_node() does point out
the limitation that it shouldn't be shared with other devices.

Now, that's rather fragile and the reason I pointed out [1] it should be
changed, but it's technically not a fix?

[1] https://lore.kernel.org/all/DM1FMM3KEYU9.1D26IINII93FO@kernel.org/

>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>> ---
>> Hi!
>> 
>> This is my approach at addressing the bug reported by Vivekanandan
>> directly in driver core. Please test it if you can.
>
> This looks good to me, but I think the patch should be simplified somewhat.
>
>> @@ -1190,6 +1191,7 @@ void software_node_notify(struct device *dev)
>>  void software_node_notify_remove(struct device *dev)
>>  {
>>  	struct swnode *swnode;
>> +	bool is_owner;
>>  
>>  	swnode = dev_to_swnode(dev);
>>  	if (!swnode)
>> @@ -1206,9 +1208,13 @@ void software_node_notify_remove(struct device *dev)
>>  	if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
>>  		swnode->fwnode.dev = NULL;
>>  
>> +	is_owner = swnode->owner && swnode->owner == dev;
>
> The NULL check here is redundant.
>
>> +	if (is_owner)
>> +		swnode->owner = NULL;
>
> There is no need to clear the owner field as devices are not allowed to be
> re-registered.

I guess that's true, but maybe leaving in a defined state is a bit more robust?

>> +
>>  	swnode_put(swnode);
>>  
>> -	if (swnode->managed) {
>> +	if (is_owner) {
>>  		set_secondary_fwnode(dev, NULL);
>>  		swnode_put(swnode);
>>  	}
>
> And if we drop the managed reference before the notify one, we can drop
> the temporary as well and this chunk just becomes:
>
> @@ -1206,12 +1207,12 @@ void software_node_notify_remove(struct device *dev)
>         if (!device_match_fwnode(dev, &swnode->fwnode) && swnode->fwnode.dev == dev)
>                 swnode->fwnode.dev = NULL;
>  
> -       swnode_put(swnode);
> -
> -       if (swnode->managed) {
> +       if (swnode->owner == dev) {
>                 set_secondary_fwnode(dev, NULL);
>                 swnode_put(swnode);
>         }
> +
> +       swnode_put(swnode);
>  }
>
> The end-result applied for 7.4 (to fix the 7.3 regression) is here:

A previous reply from Balasubramani suggested to adjust the doc-comment of
device_create_managed_software_node(), so I thought Bartosz will send a new
version.

Also, are we sure this is -rc material? I think the original commit wasn't
technically broken and the bug in Xe was fixed otherwise.

Thanks,
Danilo