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

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 results in a warning like:

  proc_dir_entry '000c:00/00.0' already registered

The caller receives NULL (duplicate entry) and unconditionally stores it to
bus->procdir, corrupting the valid pointer set by the first caller.

Serialise access to proc_initialized, proc_bus_pci_dir, bus->procdir and
dev->procent with a new mutex local to drivers/pci/proc.c, and store the
created entries to bus->procdir and dev->procent only on success, so a
failed creation can never overwrite a valid pointer.

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().

Reported-by: Shuan He <heshuan@bytedance.com>
Closes: https://lore.kernel.org/linux-pci/20250702155112.40124-2-heshuan@bytedance.com/
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://lore.kernel.org/r/20260611150543.511422-1-kwilczynski@kernel.org

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index ce36e35681e8e75d38128a41abbf926c68702c8a..71ad289fcb8e3126f0ddcce9ac795b5f56e6c18c 100644 (file)
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -18,6 +18,7 @@
 #include "pci.h"
 
 static int proc_initialized;   /* = 0 */
+static DEFINE_MUTEX(pci_proc_lock);
 
 static loff_t proc_bus_pci_lseek(struct file *file, loff_t off, int whence)
 {
@@ -416,40 +417,64 @@ static const struct seq_operations proc_bus_pci_devices_op = {
 
 static struct proc_dir_entry *proc_bus_pci_dir;
 
-int pci_proc_attach_device(struct pci_dev *dev)
+static int __pci_proc_attach_bus(struct pci_bus *bus)
 {
-       struct pci_bus *bus = dev->bus;
-       struct proc_dir_entry *e;
+       struct proc_dir_entry *dir;
        char name[16];
 
+       lockdep_assert_held(&pci_proc_lock);
+
        if (!proc_initialized)
                return -EACCES;
 
-       if (!bus->procdir) {
-               if (pci_proc_domain(bus)) {
-                       sprintf(name, "%04x:%02x", pci_domain_nr(bus),
-                                       bus->number);
-               } else {
-                       sprintf(name, "%02x", bus->number);
-               }
-               bus->procdir = proc_mkdir(name, proc_bus_pci_dir);
-               if (!bus->procdir)
-                       return -ENOMEM;
-       }
+       if (bus->procdir)
+               return 0;
+
+       if (pci_proc_domain(bus))
+               sprintf(name, "%04x:%02x", pci_domain_nr(bus), bus->number);
+       else
+               sprintf(name, "%02x", bus->number);
+
+       dir = proc_mkdir(name, proc_bus_pci_dir);
+       if (!dir)
+               return -ENOMEM;
+
+       bus->procdir = dir;
+
+       return 0;
+}
+
+int pci_proc_attach_device(struct pci_dev *dev)
+{
+       struct pci_bus *bus = dev->bus;
+       struct proc_dir_entry *entry;
+       char name[16];
+       int ret;
+
+       guard(mutex)(&pci_proc_lock);
+
+       if (dev->procent)
+               return 0;
+
+       ret = __pci_proc_attach_bus(bus);
+       if (ret)
+               return ret;
 
        sprintf(name, "%02x.%x", PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
-       e = proc_create_data(name, S_IFREG | S_IRUGO | S_IWUSR, bus->procdir,
-                            &proc_bus_pci_ops, dev);
-       if (!e)
+       entry = proc_create_data(name, S_IFREG | S_IRUGO | S_IWUSR,
+                                bus->procdir, &proc_bus_pci_ops, dev);
+       if (!entry)
                return -ENOMEM;
-       proc_set_size(e, dev->cfg_size);
-       dev->procent = e;
+
+       proc_set_size(entry, dev->cfg_size);
+       dev->procent = entry;
 
        return 0;
 }
 
 int pci_proc_detach_device(struct pci_dev *dev)
 {
+       guard(mutex)(&pci_proc_lock);
        proc_remove(dev->procent);
        dev->procent = NULL;
        return 0;
@@ -457,19 +482,27 @@ int pci_proc_detach_device(struct pci_dev *dev)
 
 int pci_proc_detach_bus(struct pci_bus *bus)
 {
+       guard(mutex)(&pci_proc_lock);
        proc_remove(bus->procdir);
+       bus->procdir = NULL;
        return 0;
 }
 
 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_initialized = 1;
+
+       scoped_guard(mutex, &pci_proc_lock) {
+               proc_bus_pci_dir = proc_mkdir("bus/pci", NULL);
+               proc_create_seq("devices", 0, proc_bus_pci_dir,
+                               &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;
 }