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

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

[Krzysztof Wilczyński]

pci_proc_attach_device() creates procfs entries for PCI devices and is
called from pci_bus_add_device().  It 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() creates procfs entries for
these already-enumerated devices.  This loop runs without holding any
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, leading to duplicate proc_create_data() calls.

Thus, wrap the for_each_pci_dev() loop 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, making the
function idempotent and symmetric with pci_proc_detach_device() which
clears this field.

While at it, update code to match preferred style and help with
readability.

Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
---
 drivers/pci/proc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index ce36e35681e8..9326f235bc58 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -425,6 +425,9 @@ int pci_proc_attach_device(struct pci_dev *dev)
 	if (!proc_initialized)
 		return -EACCES;
 
+	if (dev->procent)
+		return 0;
+
 	if (!bus->procdir) {
 		if (pci_proc_domain(bus)) {
 			sprintf(name, "%04x:%02x", pci_domain_nr(bus),
@@ -464,12 +467,17 @@ int pci_proc_detach_bus(struct pci_bus *bus)
 static int __init pci_proc_init(void)
 {
 	struct pci_dev *dev = NULL;
+
 	proc_bus_pci_dir = proc_mkdir("bus/pci", NULL);
 	proc_create_seq("devices", 0, proc_bus_pci_dir,
-		    &proc_bus_pci_devices_op);
+			&proc_bus_pci_devices_op);
+
 	proc_initialized = 1;
+
+	pci_lock_rescan_remove();
 	for_each_pci_dev(dev)
 		pci_proc_attach_device(dev);
+	pci_unlock_rescan_remove();
 
 	return 0;
 }
-- 
2.54.0

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

[Krzysztof Wilczyński]

Hello,

> Thus, wrap the for_each_pci_dev() loop 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, making the
> function idempotent and symmetric with pci_proc_detach_device() which
> clears this field.

A note on testing:

  0-day bot (recent test runs; newer builds will arrive later):
    - https://lore.kernel.org/linux-pci/202603162306.2oKy0qcP-lkp@intel.com

  Sashiko's feedback:
    - https://sashiko.dev/#/patchset/20260430003542.455198-1-kwilczynski%40kernel.org

Lorenzo Pieralisi did some testing reported outside the mailing list (we
talked on IRC) on the platform he had some boot issues.  With this patch
applied, the problems seen before were resolved.

Thank you!

	Krzysztof

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

[Bjorn Helgaas]

On Fri, May 01, 2026 at 10:22:56AM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> > Thus, wrap the for_each_pci_dev() loop 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, making the
> > function idempotent and symmetric with pci_proc_detach_device() which
> > clears this field.
> 
> A note on testing:
> 
>   0-day bot (recent test runs; newer builds will arrive later):
>     - https://lore.kernel.org/linux-pci/202603162306.2oKy0qcP-lkp@intel.com
> 
>   Sashiko's feedback:
>     - https://sashiko.dev/#/patchset/20260430003542.455198-1-kwilczynski%40kernel.org
> 
> Lorenzo Pieralisi did some testing reported outside the mailing list (we
> talked on IRC) on the platform he had some boot issues.  With this patch
> applied, the problems seen before were resolved.

Thanks!  Can we include a link to the problem report and maybe a
couple lines of the symptoms?

Also the analysis of Sashiko feedback?

Sashiko worried about pci_lock_rescan_remove() deadlock between
pci_proc_init() and PCI controller drivers with async probing.
pci_proc_init() is a device initcall.  Some drivers are also device
initcalls (imx_pcie_init(), ks_pcie_init(), rcar_pcie_init()), and it
looks like they can use async probing.

Does this rely on the pci_proc_init() device_initcall happening before
any of the driver device_initcalls?  That would be non-obvious and
fragile.

The second sashiko issue (concurrent calls of
pci_proc_attach_device()) also seems worth a look.  The
pci_enable_sriov() path isn't serialized by pci_lock_rescan_remove():

  pci_enable_sriov
    sriov_enable
      sriov_add_vfs
        pci_iov_add_virtfn
          pci_bus_add_device
            pci_proc_attach_device
              bus->procdir = proc_mkdir()

If two threads race for devices on the same bus, it looks like the
loser can set bus->procdir back to NULL when proc_mkdir() fails with
"duplicate entry".

This is a per-device path, but we're creating a per-bus directory.  I
wonder if that proc_mkdir() could/should be done in a bus creation
path?

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

[Lorenzo Pieralisi]

On Fri, May 01, 2026 at 02:37:21PM -0500, Bjorn Helgaas wrote:
> On Fri, May 01, 2026 at 10:22:56AM +0900, Krzysztof Wilczyński wrote:
> > Hello,
> > 
> > > Thus, wrap the for_each_pci_dev() loop 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, making the
> > > function idempotent and symmetric with pci_proc_detach_device() which
> > > clears this field.
> > 
> > A note on testing:
> > 
> >   0-day bot (recent test runs; newer builds will arrive later):
> >     - https://lore.kernel.org/linux-pci/202603162306.2oKy0qcP-lkp@intel.com
> > 
> >   Sashiko's feedback:
> >     - https://sashiko.dev/#/patchset/20260430003542.455198-1-kwilczynski%40kernel.org
> > 
> > Lorenzo Pieralisi did some testing reported outside the mailing list (we
> > talked on IRC) on the platform he had some boot issues.  With this patch
> > applied, the problems seen before were resolved.
> 
> Thanks!  Can we include a link to the problem report and maybe a
> couple lines of the symptoms?

You can add a Link tag:

https://lore.kernel.org/linux-pci/20250702155112.40124-2-heshuan@bytedance.com/

FWIW I tested it on top of:

https://lore.kernel.org/lkml/20260505-gic-v5-acpi-iwb-probe-deferral-v1-0-b37b85998362@kernel.org/

so:

Tested-by: Lorenzo Pieralisi <lpieralisi@kernel.org>

> Also the analysis of Sashiko feedback?

I have not looked into this yet though I/We should.

Thanks for the fix !
Lorenzo

> Sashiko worried about pci_lock_rescan_remove() deadlock between
> pci_proc_init() and PCI controller drivers with async probing.
> pci_proc_init() is a device initcall.  Some drivers are also device
> initcalls (imx_pcie_init(), ks_pcie_init(), rcar_pcie_init()), and it
> looks like they can use async probing.
> 
> Does this rely on the pci_proc_init() device_initcall happening before
> any of the driver device_initcalls?  That would be non-obvious and
> fragile.
> 
> The second sashiko issue (concurrent calls of
> pci_proc_attach_device()) also seems worth a look.  The
> pci_enable_sriov() path isn't serialized by pci_lock_rescan_remove():
> 
>   pci_enable_sriov
>     sriov_enable
>       sriov_add_vfs
>         pci_iov_add_virtfn
>           pci_bus_add_device
>             pci_proc_attach_device
>               bus->procdir = proc_mkdir()
> 
> If two threads race for devices on the same bus, it looks like the
> loser can set bus->procdir back to NULL when proc_mkdir() fails with
> "duplicate entry".
> 
> This is a per-device path, but we're creating a per-bus directory.  I
> wonder if that proc_mkdir() could/should be done in a bus creation
> path?

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

[Krzysztof Wilczyński]

Hello,

> > > Lorenzo Pieralisi did some testing reported outside the mailing list (we
> > > talked on IRC) on the platform he had some boot issues.  With this patch
> > > applied, the problems seen before were resolved.
> > 
> > Thanks!  Can we include a link to the problem report and maybe a
> > couple lines of the symptoms?
> 
> You can add a Link tag:
> 
> https://lore.kernel.org/linux-pci/20250702155112.40124-2-heshuan@bytedance.com/
> 
> FWIW I tested it on top of:
> 
> https://lore.kernel.org/lkml/20260505-gic-v5-acpi-iwb-probe-deferral-v1-0-b37b85998362@kernel.org/
> 
> so:
> 
> Tested-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> 
> > Also the analysis of Sashiko feedback?
> 
> I have not looked into this yet though I/We should.
> 
> Thanks for the fix !

Thank you for taking the time to test it!  Much appreciated.

	Krzysztof