[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

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

[Bartosz Golaszewski]

On Mon, Nov 09, 2026 at 03:10:31PM +0100, Danilo Krummrich wrote:
> 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/
>

I don't mind dropping the tag.

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

I will, I was just giving it some time on the list.

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

We can queue it for v7.5 alright.

Bart

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

[Johan Hovold]

On Mon, Nov 09, 2026 at 03:10:31PM +0100, Danilo Krummrich wrote:
> 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/

It's hard to determine at allocation time whether a node will eventually
end up being reused. That's why added the following clarification to the
commit message:

	Note that it was never the intention that managed software nodes
	would be shared, but descendants reusing ancestor firmware nodes
	is not uncommon and such use is hard to predict when allocating
	software nodes.
 
> >> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

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

I missed that comment (and his Reviewed-by tag, will add).

The function still does not return the allocated node and managed nodes
should generally not be shared. The exception would be descendants
reusing ancestor nodes, and that may happen elsewhere in the stack.

So I think the kernel doc is fine as-is, for now.

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

The regression was introduced by 0f485f88656a ("drm/xe/i2c: use
device_create_managed_software_node()") in 7.3 and Bartosz framed it as
an alternative fix for that bug.

Are you saying this has since been fixed differently?

If so, this can obviously wait.

Johan

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

[Danilo Krummrich]

On Mon Nov 9, 2026 at 3:34 PM CET, Johan Hovold wrote:
> On Mon, Nov 09, 2026 at 03:10:31PM +0100, Danilo Krummrich wrote:
>> 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/
>
> It's hard to determine at allocation time whether a node will eventually
> end up being reused. That's why added the following clarification to the
> commit message:
>
> 	Note that it was never the intention that managed software nodes
> 	would be shared, but descendants reusing ancestor firmware nodes
> 	is not uncommon and such use is hard to predict when allocating
> 	software nodes.

Yeah, that's why I said it's fragile. :)

(Not trying to split hairs, I guess both can be argued for, whether we treat
something like this as fix or not, so I'm fine with either.)

>> >> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>
>> > 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.
>
> I missed that comment (and his Reviewed-by tag, will add).
>
> The function still does not return the allocated node and managed nodes
> should generally not be shared. The exception would be descendants
> reusing ancestor nodes, and that may happen elsewhere in the stack.
>
> So I think the kernel doc is fine as-is, for now.

That's fair.

>> 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.
>
> The regression was introduced by 0f485f88656a ("drm/xe/i2c: use
> device_create_managed_software_node()") in 7.3 and Bartosz framed it as
> an alternative fix for that bug.
>
> Are you saying this has since been fixed differently?
>
> If so, this can obviously wait.

There's the fix in [1] and Andy said he'll pick it up for Xe in one of the
replies.

[1] https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/

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

[Johan Hovold]

On Mon, Nov 09, 2026 at 03:45:29PM +0100, Danilo Krummrich wrote:
> On Mon Nov 9, 2026 at 3:34 PM CET, Johan Hovold wrote:
> > On Mon, Nov 09, 2026 at 03:10:31PM +0100, Danilo Krummrich wrote:
> >> 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()")

> >> 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.
> >
> > The regression was introduced by 0f485f88656a ("drm/xe/i2c: use
> > device_create_managed_software_node()") in 7.3 and Bartosz framed it as
> > an alternative fix for that bug.
> >
> > Are you saying this has since been fixed differently?
> >
> > If so, this can obviously wait.
> 
> There's the fix in [1] and Andy said he'll pick it up for Xe in one of the
> replies.

Yeah, I skimmed the thread now. I think the last thing he said was:

	The patch looks fine to me but I still see that there are some
	concerns. Like Danilo's points are valid, can those be addressed
	later?

	...

	Let's wait a little and if nothing arises I can apply this for
	XE.

and in the reply they say would go back to managed nodes "once the
limitation is addressed".

The proposed xe fix is still not in linux-next however, and this patch
carries a Closes: tag for the regression.

So I think we can just go ahead and fix this in driver core directly.

Johan

> [1] https://lore.kernel.org/all/20261009164241.923756-2-balasubramani.vivekanandan@intel.com/