Linux Media Controller development
 help / color / mirror / Atom feed
* ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
@ 2026-08-27 23:26 D. Manresa
  2026-08-28 15:33 ` Sakari Ailus
  0 siblings, 1 reply; 3+ messages in thread
From: D. Manresa @ 2026-08-27 23:26 UTC (permalink / raw)
  To: Sakari Ailus, Bingbu Cao, Dan Scally
  Cc: D . Manresa, Tianshu Qiu, Mauro Carvalho Chehab, linux-media,
	linux-kernel

Hi,

ipu-bridge registers software nodes at probe time but has no teardown
path at all: nothing unregisters them when the IPU device goes away.
After a PCI remove/rescan of the IPU6, re-probe fails with -EEXIST,
the orphaned nodes stay in /sys/kernel/software_nodes/, and once the
module has been unloaded their properties reference freed module
memory, so sensor drivers that re-probe read garbage. There is no
userspace interface to remove software nodes; only a reboot recovers.

Hardware / kernel
-----------------
- Microsoft Surface Pro 7+, IPU6 Tiger Lake (PCI 8086:9a19, 0000:00:05.0)
- sensors OV5693 (INT33BE), OV8865 (INT347A, dw9719 VCM), OV7251
  (INT347E)
- linux-surface 6.19.8-surface-3 (base v6.19.8). The linux-surface
  patchset only adds entries to ipu_supported_sensors[]; the
  registration/teardown paths discussed here are unmodified mainline.
  Line references are against mainline v6.19.

Analysis
--------
ipu_bridge_init() (drivers/media/pci/intel/ipu-bridge.c:837), called
from ipu6_isys_init() (drivers/media/pci/intel/ipu6/ipu6.c:378) during
probe:

- kzallocs struct ipu_bridge (ipu-bridge.c:854); on success it is
  intentionally never freed -- the comment at ipu-bridge.c:870 keeps
  data in the struct "so that it survives if the module is unloaded
  along with the rest of the struct";
- registers the IPU HID node "INT343E" (IPU_HID,
  include/media/ipu-bridge.h:11) via software_node_register()
  (ipu-bridge.c:864);
- registers one node group per sensor via
  software_node_register_node_group() in ipu_bridge_connect_sensor()
  (ipu-bridge.c:729);
- points the sensor ACPI fwnode at the swnodes: primary->secondary =
  fwnode (ipu-bridge.c:743), and set_secondary_fwnode(dev, fwnode) for
  the IPU itself (ipu-bridge.c:893).

The only unregister calls in the file are error paths inside init
itself: ipu_bridge_unregister_sensors() (static, ipu-bridge.c:681,
called at 784 and 898) and software_node_unregister(
&bridge->ipu_hid_node) (ipu-bridge.c:900). No teardown function is
exported, and nothing on the ipu6 remove path unregisters the nodes or
clears the secondary fwnodes.

Because the swnodes and their property_entry arrays live in the leaked
heap struct they survive unbind, but they are not self-contained:

- the property name strings are pointers into the module image
  (sensor->prop_names = prop_names at ipu-bridge.c:367; static const
  prop_names at ipu-bridge.c:124);
- the "link-frequencies" values point at cfg->link_freqs inside the
  module-rodata table ipu_supported_sensors[] (property built at
  ipu-bridge.c:433, table at ipu-bridge.c:51).

So after ipu_bridge.ko is unloaded, the still-registered nodes carry
dangling pointers, and the sensors' ACPI fwnodes still have
fwnode->secondary aimed at them.

Observed behaviour (live, 2026-08-27)
-------------------------------------
Attempting to recover a wedged ISYS by PCI remove/rescan:

  echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
  modprobe -r intel_ipu6_isys intel_ipu6    # ipu_bridge unloads too
  echo 1 > /sys/bus/pci/rescan
  modprobe intel_ipu6

dmesg (verbatim, trimmed):

  sysfs: cannot create duplicate filename
   '/kernel/software_nodes/INT343E'
  Call Trace:
   sysfs_warn_dup+0x8b/0xb0
   ...
   swnode_register+0x165/0x240
   software_node_register+0xd2/0x120
   ipu_bridge_init+0x192/0xeb0 [ipu_bridge]
   ipu6_pci_probe+0x417/0xbe0 [intel_ipu6]
   ...
  kobject: kobject_add_internal failed for INT343E with -EEXIST, don't
   try to register things with the same name in the same directory.
  intel-ipu6 0000:00:05.0: Failed to register the IPU HID node
  intel-ipu6 0000:00:05.0: error -EEXIST: IPU6 bridge init failed
  intel-ipu6 0000:00:05.0: probe with driver intel-ipu6 failed with
   error -17

The sensor drivers, re-probing against the stale secondary fwnodes,
then read poisoned link-frequencies (verbatim):

  ov5693 i2c-INT33BE:00: supported link freq 419200000ll not found
  ov5693 i2c-INT33BE:00: probe with driver ov5693 failed with error -22
  ov7251 i2c-INT347E:00: error -EINVAL: no supported link freq found
  ov7251 i2c-INT347E:00: probe with driver ov7251 failed with error -22
  ov8865 i2c-INT347A:00: failed to find 360000000 clk rate in endpoint
   link-frequencies

(The ov5693/ov7251 on this machine carry local patches, so the exact
wording of their two lines may differ from pure mainline; the ov8865
line is the unmodified mainline driver, and the values being missed --
419200000, 360000000 -- are exactly what the bridge publishes for
these sensors at ipu-bridge.c:61 and :69, so the properties no longer
return their original contents.)

/sys/kernel/software_nodes/ afterwards still lists INT343E, INT33BE-4,
INT347A-1, INT347E-5 and dw9719-1. There is no userspace interface to
unregister a software node, so the system cannot be recovered without
a reboot.

Reproducer
----------
On any IPU6 machine handled by ipu-bridge:

  1. boot normally, wait for "Connected N cameras"
  2. echo 1 > /sys/bus/pci/devices/0000:00:05.0/remove
  3. modprobe -r intel_ipu6_isys intel_ipu6
  4. echo 1 > /sys/bus/pci/rescan
  5. modprobe intel_ipu6

Step 5 (or the automatic probe at step 4) fails with the -EEXIST trace
above and the cameras are gone until reboot.

Expected: either an idempotent/refcounted ipu_bridge_init() that
reuses already-registered nodes across rebinds, or a real teardown
that unregisters the node groups and clears the secondary fwnodes on
unbind (at which point the deliberate struct leak could go as well).

Workaround: none other than not touching the PCI device; documented as
an operational trap in
https://github.com/dmanresa-saes/surface-ipu6-cameras

Happy to test patches on this hardware.

This report was drafted with AI assistance (Anthropic Claude) and
verified on the actual hardware by the undersigned.

D. Manresa <dmanresa@gmail.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
  2026-08-27 23:26 ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties D. Manresa
@ 2026-08-28 15:33 ` Sakari Ailus
  2026-08-28 20:54   ` D. Manresa
  0 siblings, 1 reply; 3+ messages in thread
From: Sakari Ailus @ 2026-08-28 15:33 UTC (permalink / raw)
  To: D. Manresa
  Cc: Bingbu Cao, Dan Scally, Tianshu Qiu, Mauro Carvalho Chehab,
	linux-media, linux-kernel

Hi D.,

On Fri, Aug 28, 2026 at 01:26:36AM +0200, D. Manresa wrote:
> Hi,
> 
> ipu-bridge registers software nodes at probe time but has no teardown
> path at all: nothing unregisters them when the IPU device goes away.
> After a PCI remove/rescan of the IPU6, re-probe fails with -EEXIST,
> the orphaned nodes stay in /sys/kernel/software_nodes/, and once the
> module has been unloaded their properties reference freed module
> memory, so sensor drivers that re-probe read garbage. There is no
> userspace interface to remove software nodes; only a reboot recovers.
> 
> Hardware / kernel
> -----------------
> - Microsoft Surface Pro 7+, IPU6 Tiger Lake (PCI 8086:9a19, 0000:00:05.0)
> - sensors OV5693 (INT33BE), OV8865 (INT347A, dw9719 VCM), OV7251
>   (INT347E)
> - linux-surface 6.19.8-surface-3 (base v6.19.8). The linux-surface
>   patchset only adds entries to ipu_supported_sensors[]; the
>   registration/teardown paths discussed here are unmodified mainline.
>   Line references are against mainline v6.19.

The regulator string length has been increased to 6 since and I believe
with <20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm> (on LMML) the
camera might work streaming-wise at least. There might be Bayer order
issues though, there was another patchset
(<20260729-sp7plus-ov-flips-v2-0-91884b81a8f5@berg.pm>) addressing those.

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
  2026-08-28 15:33 ` Sakari Ailus
@ 2026-08-28 20:54   ` D. Manresa
  0 siblings, 0 replies; 3+ messages in thread
From: D. Manresa @ 2026-08-28 20:54 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: D . Manresa, Dan Scally, Mauro Carvalho Chehab, linux-media,
	linux-kernel

Hi Sakari,

On Fri, Aug 28, 2026 at 06:33:16PM +0300, Sakari Ailus wrote:
> The regulator string length has been increased to 6 since and I believe
> with <20260729-sp7plus-int3472-v2-1-cdfaf97ac3ad@berg.pm> (on LMML) the
> camera might work streaming-wise at least. There might be Bayer order
> issues though, there was another patchset
> (<20260729-sp7plus-ov-flips-v2-0-91884b81a8f5@berg.pm>) addressing those.

Thanks for the pointers -- good to know about the string length fix, and
we are aware of Jakob's two series: we have adopted his POWER1 approach
in our local tree and have offered him Tested-by on this hardware to
help both series move.

This report is orthogonal to the sensor bring-up, though: all three
sensors here are fully powered and streaming (the cameras work
end-to-end, hardware ISP included). The issue is that ipu-bridge has no
teardown -- after any unbind of the IPU PCI device its software nodes
stay registered, so a rescan can never re-probe (-EEXIST), and once the
module is gone the node properties dangle into freed module memory. It
reproduces on any ipu-bridge machine regardless of sensor state.

Would a fix be welcome, and if so, which direction do you prefer:
a real teardown on unbind (unregister the node groups, clear the
secondary fwnodes, drop the deliberate struct leak), or making
ipu_bridge_init() idempotent so a rebind reuses the already-registered
nodes? Happy to write and test either on this hardware.

Regards,

D. Manresa

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-28 20:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 23:26 ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties D. Manresa
2026-08-28 15:33 ` Sakari Ailus
2026-08-28 20:54   ` D. Manresa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox