All of lore.kernel.org
 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: 17+ 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
2026-08-30 12:40     ` johannes.goede
2026-08-31  8:02       ` Sakari Ailus
2026-08-31 10:23         ` D. Manresa
2026-08-31 11:36           ` Sakari Ailus
2026-08-31  9:03 ` Sakari Ailus
2026-08-31  9:42 ` [PATCH 0/2] media: ipu-bridge: survive module unload and reuse the software nodes on rebind D. Manresa
2026-08-31  9:42   ` [PATCH 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-08-31  9:42   ` [PATCH 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2026-08-31 12:11     ` Sakari Ailus
2026-08-31 14:03   ` [PATCH v2 0/2] media: ipu-bridge: survive module unload and " D. Manresa
2026-08-31 14:03     ` [PATCH v2 1/2] media: ipu-bridge: don't reference the module image from software nodes D. Manresa
2026-08-31 14:03     ` [PATCH v2 2/2] media: ipu-bridge: reuse the software nodes on rebind D. Manresa
2026-09-01 10:47       ` Sakari Ailus
2026-09-01 11:25         ` 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.