Linux Media Controller development
 help / color / mirror / Atom feed
From: "D. Manresa" <dmanresa@gmail.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Bingbu Cao <bingbu.cao@intel.com>,
	Dan Scally <dan.scally@ideasonboard.com>
Cc: "D . Manresa" <dmanresa@gmail.com>,
	Tianshu Qiu <tian.shu.qiu@intel.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties
Date: Fri, 28 Aug 2026 01:26:36 +0200	[thread overview]
Message-ID: <20260827232636.93145-1-dmanresa@gmail.com> (raw)

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>

             reply	other threads:[~2026-08-27 23:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27 23:26 D. Manresa [this message]
2026-08-28 15:33 ` ipu-bridge: software nodes are never unregistered; PCI remove/rescan of IPU6 fails with -EEXIST and leaves dangling properties Sakari Ailus
2026-08-28 20:54   ` D. Manresa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260827232636.93145-1-dmanresa@gmail.com \
    --to=dmanresa@gmail.com \
    --cc=bingbu.cao@intel.com \
    --cc=dan.scally@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=tian.shu.qiu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox