media: ipu-bridge: lifetime of retained software-node properties

media: ipu-bridge: lifetime of retained software-node properties

[Ruslan Koreev]

Hi Sakari, Dan,

While moving the LM3643 board description into ipu-bridge, I noticed an
existing property lifetime issue in media next d7e7bca71698.

The bridge deliberately keeps its software-node graph after module unload,
but link-frequencies still points at cfg->link_freqs in module storage.
The lens-focus property name is also a module string literal.
software_node_register() retains these pointers without copying them.

This predates my local flash integration, whose new nodes own their backing
storage. This finding is from source review; I have not yet tested unloading
a populated sensor graph.

I propose a separate media fix that copies the remaining names and values
into graph-owned storage, with a VM unload/reload test. Would you prefer
that as a prerequisite to the flash integration, or handled independently?

Regards,
Ruslan

Re: media: ipu-bridge: lifetime of retained software-node properties

[Fernando Rimoli]

Hi Ruslan,

On Mon, Oct 26, 2026 at 12:12:52PM +0300, Ruslan Koreev wrote:
> The bridge deliberately keeps its software-node graph after module unload,
> but link-frequencies still points at cfg->link_freqs in module storage.
> The lens-focus property name is also a module string literal.

Both are covered already. D. Manresa's series, whose v5 went out this
morning (Cc'd), fixes exactly these two in 1/2: the lens-focus name goes
into struct ipu_property_names and the link frequencies are copied into
struct ipu_sensor.

https://lore.kernel.org/linux-media/20261026082549.662885-1-dmanresa@gmail.com/

It follows a20c843c9bc4 ("media: ipu-bridge: Keep the
clock-noncontinuous property name out of rodata"), which did the same
for clock-noncontinuous. As far as I can see, with those two gone
nothing in the graph points into the module any more and every other name
and value is already a per-sensor copy. Sergey Lebedev's VD55G0 series
adds an "st,leds" property, and his v2 will store it the same way (Cc'd
too).

So there may be no need for a separate fix. Basing the flash integration
on Manresa's 1/2, and putting any new names and values in the same
places, would keep it consistent.

Fernando

Re: media: ipu-bridge: lifetime of retained software-node properties

[D. Manresa]

On Mon, Oct 26, 2026 at 11:11:06AM +0000, Fernando Rimoli wrote:
> As far as I can see, with those two gone nothing in the graph points
> into the module any more and every other name and value is already a
> per-sensor copy.

I checked that instead of just agreeing with it, since Ruslan and Sergey
are both about to add properties and it is a bad thing to get wrong. It
holds, with one trap worth recording.

Against media/next with v5 applied, in ipu-bridge.c:

- every registered node's .name is a per-sensor or per-bridge char
  array: sensor->name, sensor->ivsc_name, sensor->node_names.*, and
  bridge->ipu_node_name for the IPU HID node itself;
- every property name comes from sensor->prop_names, a struct
  ipu_property_names of char arrays copied per sensor;
- every property value is an inline scalar, bridge->data_lanes,
  sensor->link_freqs (what 1/2 changes) or one of the sensor->*_ref
  arrays;
- there is no PROPERTY_ENTRY_STRING anywhere, so no property holds a
  string pointer as a value;
- .properties and .parent all point inside struct ipu_sensor or at
  bridge->ipu_hid_node.

All of that lives in the struct ipu_bridge that ipu_bridge_init()
allocates and then deliberately never frees: kfree(bridge) appears only
on the error paths, and there is no module_exit() or remove hook. So the
precise statement is not that nothing points into the module, but that
everything points into a heap allocation that outlives it. That is also
the answer to where a new name or value should go.

The trap: sensor->vcm_type still points into ipu_vcm_types[], which is
module rodata, and it survives in the retained struct. It is not a bug
today, because it is only read during init, and the one thing that runs
later -- ipu_bridge_instantiate_vcm() -- takes the VCM type from "%pfwP"
of the fwnode, that is from sensor->node_names.vcm, not from vcm_type.
But it is a dangling pointer sitting in a struct that outlives the
module, and the next person to read it from a rebind path will not
expect that. Either a comment or copying it like everything else would
close it.

If it helps: I have a Surface Pro 7+ (IPU6 Tiger Lake, three sensors on
the bridge) here for a few more days and can exercise the unload and the
PCI remove/rescan paths on it, which is where all of this actually
bites. Ask while it is still here.

D. Manresa

(Written with AI assistance, directed and reviewed by me. Resent as
plain text: the first copy was rejected by the list for carrying an HTML
part, so it reached the Cc list but never the archive. Apologies to
those getting it twice.)

Re: media: ipu-bridge: lifetime of retained software-node properties

[Ruslan Koreev]

Hi Fernando, Dario,

I've incorporated v5 1/2 into the local flash integration.

With both v5 patches, QEMU tests passed 16 PCI remove/rescan cycles per
run, eight including bridge unload/reload, for both the original bridge
and our integration. Frequencies and lens references remained readable
while the bridge was unloaded. No KASAN or lockdep reports.

A synthetic X1 configuration also preserved the IR flash reference, LED
string property and limits through the same lifecycle tests.

Agreed on vcm_type: it is currently init-only, and I'd keep that cleanup
separate.

Could you confirm that capture works again on each previously working
sensor after module reload and repeated PCI remove/rescan with the full
v5 series? If you've already covered this, those results would be useful.

Regards,
Ruslan

Re: media: ipu-bridge: lifetime of retained software-node properties

[D. Manresa]

Hi Ruslan,

On Tue, Oct 27, 2026 at 05:30:23PM +0300, Ruslan Koreev wrote:
> Could you confirm that capture works again on each previously working
> sensor after module reload and repeated PCI remove/rescan with the full
> v5 series?

Confirmed on the hardware. All three sensors on this Surface Pro 7+
(IPU6 Tiger Lake, 8086:9a19) delivered frames after a module reload and
after each of 8 consecutive PCI remove/rescan cycles, with both v5
patches applied. No cycle failed and no reboot was needed.

  stage                  ov5693 front  ov8865 rear  ov7251 IR
  baseline                   frames       frames      frames
  v5 fresh load              frames       frames      frames
  after module reload        frames       frames      frames
  PCI cycles 1 through 8     frames       frames      frames

I am quoting frames rather than frame rates on purpose: the per-stage
captures were 30 frames (10 for the IR), which is too short to measure a
rate. A separate 120-frame run on the rear with v5 installed gave 30.07
fps with no dropped buffers.

One thing worth saying before the rest, because it would have made the
whole test worthless: the module installed on this machine turned out to
be v4, not v5. I built v5 from the patches as posted, and before loading
it I removed the software nodes the v4 module had registered at boot, so
v5 created all five nodes from scratch rather than inheriting v4's.

What the properties actually read with the bridge unloaded
----------------------------------------------------------

This is the part your QEMU runs and mine are both really testing, so
here are the values, read back by a probe module while ipu_bridge was
fully unloaded and the nodes had been created by v5:

  INT33BE-4 (ov5693)  link-frequencies[0] = 419200000
                      clock-frequency = 19200000, rotation = 180
  INT347A-1 (ov8865)  link-frequencies[0] = 360000000
                      lens-focus -> 'dw9719-1', reference resolves
  INT347E-5 (ov7251)  link-frequencies[0] = 319200000
                      clock-frequency = 19200000
  INT343E             data-lanes intact on port@1 (1,2,3,4),
                      port@4 (1,2), port@5 (1)

Nothing poisoned, and identical when re-read after the 8 cycles. The
lens-focus property, whose name was the module string literal, still
resolves. As a functional check of the same thing I re-modprobed
ov5693, ov8865 and ov7251 with the bridge still unloaded: all three
bound to their i2c clients with no "supported link freq not found" and
no errors.

The refcount, per cycle
-----------------------

With dynamic debug on, every cycle logged "Reusing the previously
registered software nodes" exactly once, and the INT343E kobject
refcount went

  4 (before) -> 3 (after PCI remove) -> 4 (after re-bind)

flat across all 8 cycles, with every child node still present. For
contrast, v4 measured earlier on this same machine went 4, 3, 2, 1 and
the node was gone by cycle 4.

What this test cannot tell you
------------------------------

This kernel has CONFIG_KASAN, CONFIG_PROVE_LOCKING, CONFIG_LOCKDEP,
CONFIG_DEBUG_LIST and CONFIG_DEBUG_KOBJECT all unset, and slub_debug is
not on the command line. So the absence of a KASAN report here means
KASAN was not available, not that it was clean. The property read-back
and the refcount are the evidence; the quiet log is not. If someone has
a KASAN-enabled kernel on IPU hardware, that run is still worth doing.

A build note for anyone repeating this out of tree: v5 changes struct
ipu_sensor, so the modversion CRCs of ipu_bridge_init and
ipu_bridge_parse_ssdb change and an out-of-tree intel-ipu6 must be
rebuilt against the new symvers, otherwise modprobe fails with
"disagrees about version of symbol ipu_bridge_init". An in-tree build
never sees this. intel-ipu6-isys only imports
ipu_bridge_instantiate_vcm, whose CRC is unchanged.

sensor->vcm_type still points into module rodata in the surviving
struct, as discussed earlier in this thread. Not a live bug today, and I
agree with keeping that cleanup separate.

This machine leaves in a few days, so if there is anything else you want
exercised on real IPU6 hardware with three sensors on the bridge, now is
the time to ask.

D. Manresa

(Written with AI assistance, directed and reviewed by me.)