Re: [PATCH v3] PCI/proc: Fix race between pci_proc_init() and pci_bus_add_device()

["Krzysztof Wilczyński" <kwilczynski@kernel.org>]

Hello,

> pci_proc_attach_device() creates procfs entries for PCI devices and is
> called from pci_bus_add_device().  It lazily creates the per-bus procfs
> directory (bus->procdir) via proc_mkdir() on first use, and returns
> early if proc_initialized is not yet set.
> 
> On x86 with ACPI, PCI enumeration occurs at subsys_initcall, before
> pci_proc_init() sets proc_initialized at device_initcall.  The
> for_each_pci_dev() loop in pci_proc_init() then creates procfs entries
> for these already-enumerated devices, but runs without holding
> pci_rescan_remove_lock.
> 
> On ARM64 with devicetree, PCI host bridges probe at device_initcall.
> With async probing enabled, pci_bus_add_device() can run concurrently
> with pci_proc_init(), and both may call pci_proc_attach_device() for
> the same device or for different devices on the same bus.  As
> pci_host_probe() holds pci_rescan_remove_lock while pci_proc_init()
> does not, there is no serialisation between the two paths.
> 
> When two threads concurrently call pci_proc_attach_device() for devices
> on the same bus, both observe bus->procdir as NULL and both call
> proc_mkdir().  The proc filesystem serialises directory creation
> internally, so only one caller succeeds.  The other receives NULL
> (duplicate entry) and unconditionally stores it to bus->procdir,
> corrupting the valid pointer set by the first caller.
> 
> Thus, extract the bus procfs directory creation from
> pci_proc_attach_device() into a new pci_proc_attach_bus() function,
> and call it from the two bus creation paths: pci_register_host_bridge()
> for root buses and pci_alloc_child_bus() for child buses.  These are
> the only two callers of pci_alloc_bus(), so for buses created after
> proc_initialized is set, bus->procdir is in place before any device
> can be added to the bus.
> 
> Therefore, by the time pci_proc_attach_device() runs on these buses,
> bus->procdir is already set and the racy proc_mkdir() call is never
> reached.
> 
> For buses created before pci_proc_init() sets proc_initialized (the
> common x86 ACPI case), the bus creation hooks return early.  As such,
> add a fallback call to pci_proc_attach_bus() from pci_proc_attach_device()
> to handle these pre-init buses.
> 
> Additionally, wrap the for_each_pci_dev() loop in pci_proc_init() with
> pci_lock_rescan_remove() to serialise against concurrent PCI bus
> operations, add an early return in pci_proc_attach_device() when
> dev->procent is already set to make the function idempotent, and clear
> bus->procdir in pci_proc_detach_bus() to prevent use of a dangling
> pointer after proc_remove().
> 
> Closes: https://lore.kernel.org/linux-pci/20250702155112.40124-2-heshuan@bytedance.com/

Applied to procfs, as I want this to have some soak time via linux-next,
plus 0-day bot will test it for me, too.

Lorenzo, do you think you could re-test this on your hardware platform?

Shuan, any chance for you to test this again on your RISC-V based platform?

Thank you!

	Krzysztof