PCI: Lock upstream bridge for pci_reset_function()

Fix a long-standing locking gap for missing pci_cfg_access_lock() while
manipulating bridge reset registers and configuration during
pci_reset_bus_function().

If there is an upstream bridge, lock it before locking the device itself.
pci_dev_lock() calls pci_cfg_access_lock(), which blocks the writing of PCI
config space by user space.

Add lockdep assertion via pci_dev->cfg_access_lock to verify
pci_dev->block_cfg_access is set.

Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Link: https://lore.kernel.org/r/20240502165851.1948523-3-dave.jiang@intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
[bhelgaas: commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 6449056b57dd3032b0a1fbd991f4248be0801d84..36f10c7f9ef56fedb5030b81091a5ae15215f14e 100644 (file)
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -275,6 +275,8 @@ void pci_cfg_access_lock(struct pci_dev *dev)
 {
        might_sleep();
 
+       lock_map_acquire(&dev->cfg_access_lock);
+
        raw_spin_lock_irq(&pci_lock);
        if (dev->block_cfg_access)
                pci_wait_cfg(dev);
@@ -329,6 +331,8 @@ void pci_cfg_access_unlock(struct pci_dev *dev)
        raw_spin_unlock_irqrestore(&pci_lock, flags);
 
        wake_up_all(&pci_cfg_wait);
+
+       lock_map_release(&dev->cfg_access_lock);
 }
 EXPORT_SYMBOL_GPL(pci_cfg_access_unlock);
 

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e5f243dd428845a124da6df6419128cdcfee201d..c63142352844177365c6695dc964fd42e928ac69 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4879,6 +4879,7 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
  */
 int pci_bridge_secondary_bus_reset(struct pci_dev *dev)
 {
+       lock_map_assert_held(&dev->cfg_access_lock);
        pcibios_reset_secondary_bus(dev);
 
        return pci_bridge_wait_for_secondary_bus(dev, "bus reset");
@@ -5245,11 +5246,20 @@ void pci_init_reset_methods(struct pci_dev *dev)
  */
 int pci_reset_function(struct pci_dev *dev)
 {
+       struct pci_dev *bridge;
        int rc;
 
        if (!pci_reset_supported(dev))
                return -ENOTTY;
 
+       /*
+        * If there's no upstream bridge, no locking is needed since there is
+        * no upstream bridge configuration to hold consistent.
+        */
+       bridge = pci_upstream_bridge(dev);
+       if (bridge)
+               pci_dev_lock(bridge);
+
        pci_dev_lock(dev);
        pci_dev_save_and_disable(dev);
 
@@ -5258,6 +5268,9 @@ int pci_reset_function(struct pci_dev *dev)
        pci_dev_restore(dev);
        pci_dev_unlock(dev);
 
+       if (bridge)
+               pci_dev_unlock(bridge);
+
        return rc;
 }
 EXPORT_SYMBOL_GPL(pci_reset_function);

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 1325fbae2f28fb10dbbbd6c6ff65ce2cf9193f3d..a3da776bf98622de15eb29cc73a9cba046170607 100644 (file)
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2543,6 +2543,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
        dev->dev.dma_mask = &dev->dma_mask;
        dev->dev.dma_parms = &dev->dma_parms;
        dev->dev.coherent_dma_mask = 0xffffffffull;
+       lockdep_register_key(&dev->cfg_access_key);
+       lockdep_init_map(&dev->cfg_access_lock, dev_name(&dev->dev),
+                        &dev->cfg_access_key, 0);
 
        dma_set_max_seg_size(&dev->dev, 65536);
        dma_set_seg_boundary(&dev->dev, 0xffffffff);

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 08b0d1d9d78b76355e76c5751a094b197a01b096..5e51b0de4c4b578a1a85a9818404d2d3470f914b 100644 (file)
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -297,6 +297,9 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
                .wait_type_inner = _wait_type,          \
                .lock_type = LD_LOCK_WAIT_OVERRIDE, }
 
+#define lock_map_assert_held(l)                \
+       lockdep_assert(lock_is_held(l) != LOCK_STATE_NOT_HELD)
+
 #else /* !CONFIG_LOCKDEP */
 
 static inline void lockdep_init_task(struct task_struct *task)
@@ -388,6 +391,8 @@ extern int lockdep_is_held(const void *);
 #define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type)    \
        struct lockdep_map __maybe_unused _name = {}
 
+#define lock_map_assert_held(l)                        do { (void)(l); } while (0)
+
 #endif /* !LOCKDEP */
 
 #ifdef CONFIG_PROVE_LOCKING

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 16493426a04ff9a2c0009644c8ae993758f34c22..e4e7b175af5477f3e4a43fa1991d9ff27a30c9f9 100644 (file)
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -413,6 +413,8 @@ struct pci_dev {
        struct resource driver_exclusive_resource;       /* driver exclusive resource ranges */
 
        bool            match_driver;           /* Skip attaching driver */
+       struct lock_class_key cfg_access_key;
+       struct lockdep_map cfg_access_lock;
 
        unsigned int    transparent:1;          /* Subtractive decode bridge */
        unsigned int    io_window:1;            /* Bridge has I/O window */