Re: [PATCH] media: ipu-bridge: hold a reference on the IVSC software node

[Bartosz Golaszewski]

On Tue, 13 Oct 2026 15:21:41 +0200, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> said:
> +Cc: Bart for the double check of the approach. This sounds like it might need
> to be fixed in driver core (software node implementation).
>
> On Sun, Oct 11, 2026 at 03:15:18PM +0200, Eduardo Ruiz Duarte wrote:
>> ipu_bridge_instantiate_ivsc() attaches the IVSC software node to the mei-csi
>> device with set_secondary_fwnode(), but never takes a reference on it, while
>> the driver core unconditionally drops one when that device goes away.
>>
>> device_add() calls software_node_notify(), which does a kobject_get() on the
>> swnode behind dev_fwnode(dev). device_del() calls
>> software_node_notify_remove(), which does the matching kobject_put() -- and
>> that put sits *outside* the "if (swnode->managed)" block, so it runs for every
>> device whose fwnode is a software node, managed or not:
>>
>> 	sysfs_remove_link(&swnode->kobj, dev_name(dev));
>> 	sysfs_remove_link(&dev->kobj, "software_node");
>> 	kobject_put(&swnode->kobj);
>>
>> 	if (swnode->managed) {
>> 		set_secondary_fwnode(dev, NULL);
>> 		kobject_put(&swnode->kobj);
>> 	}
>>
>> csi_dev is the mei-csi device, which the MEI client bus has already added by
>> the time ipu_bridge_instantiate_ivsc() runs. The add-side get therefore never
>> happens for this node, but the del-side put always does.
>>
>> On hardware where the MEI bus resets -- which it does on every s2idle and S4
>> resume on a Dell XPS 13 9340 -- mei-csi is destroyed and recreated each cycle,
>> so the node loses one reference per resume. INTC10CF-N starts at 3: one for its
>> own registration plus one for each of its two port children, since
>> kobject_add() of a child takes a reference on the parent. The third resume
>> therefore frees it, software_node_find_by_name() starts returning NULL, and
>> isys walks the fwnode graph into freed memory:
>>
>>   refcount_t: addition on 0; use-after-free
>>   WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x7e/0xb0
>>    kobject_get+0x94/0xb0
>>    software_node_graph_get_port_parent+0x89/0xd0
>>    fwnode_graph_get_port_parent+0x70/0xd0
>>    fwnode_graph_get_endpoint_by_id+0x93/0x280
>>    isys_notifier_init+0x92/0x180 [intel_ipu6_isys]
>>    isys_register_devices+0x3c9/0x4e0 [intel_ipu6_isys]
>>    isys_probe+0x3df/0x500 [intel_ipu6_isys]
>>    auxiliary_bus_probe+0x4c/0x70
>>    ...
>>    deferred_probe_work_func+0xa5/0x110
>>
>> followed by an underflow and a saturate on the same kobject, and the camera
>> never comes back.
>>
>> Take a reference so the attachment owns one, balancing the put the core will do
>> when csi_dev is destroyed. Where csi_dev is never destroyed this leaks a single
>> kobject reference, which is what the lifetime of the node already assumes.
>>
>> Found while making the camera survive hibernation on a Dell XPS 13 9340
>> (ov02c10 behind an INTC10CF IVSC); with suspend-then-hibernate the first cycle
>> burns two resumes, so the failure consistently showed up on the second
>> hibernation. Tested on 7.0.0-1015-oem: two consecutive hibernations with the
>> node alive throughout and no refcount warnings, where before the second one
>> hung the machine.
>>
>> Signed-off-by: Eduardo Ruiz Duarte <toorandom@xxxxxxxxx>
>> ---
>>  drivers/media/pci/intel/ipu-bridge.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> --- a/drivers/media/pci/intel/ipu-bridge.c
>> +++ b/drivers/media/pci/intel/ipu-bridge.c
>> @@ -765,6 +765,15 @@
>>  	if (!fwnode)
>>  		return -ENODEV;
>>
>> +	/*
>> +	 * Balance the kobject_put() that software_node_notify_remove() will do
>> +	 * on this node when csi_dev is destroyed. That put is unconditional,
>> +	 * while the matching get in software_node_notify() only happens at
>> +	 * device_add() time -- and csi_dev was added before we got here, so it
>> +	 * never ran for this node.
>> +	 */
>> +	fwnode_handle_get(fwnode);
>> +
>>  	set_secondary_fwnode(sensor->csi_dev, fwnode);
>>
>>  	return 0;
>

Thanks Andy. Yes, it looks like a bug in swnode.c. I'll send a fix proposal.

Bart