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

["Danilo Krummrich" <dakr@kernel.org>]

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