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

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

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/