[RFC PATCH 4/8] PCI/PM: Serialize pci_bridge_d3_update()

[Pavol Sakac <sakacpav@amazon.de>]

pci_bridge_d3_update() does an unlocked read-modify-write of
bridge->bridge_d3, and its callers are not mutually serialized: the
d3cold_allowed sysfs write and the driver-context D3cold helpers hold
neither pci_rescan_remove_lock nor device_lock. A concurrent write can
lose an update and leave bridge_d3 stale, costing a wrong D3cold decision
rather than memory safety. An upcoming change runs pci_bus_add_device()
for sibling VFs concurrently, making sibling additions concurrent callers
too, so this must land first.

Add a mutex around the whole update, taken once for the propagation
loop. A device with no D3cold-capable port above it returns before the
mutex, so the common add is not funneled through a global lock, and the
loop re-evaluates both conditions under it. The mutex serializes the
updaters against each other only; the d3cold_allowed store itself still
writes an adjacent bit of the same word unlocked, a pre-existing
exposure this change neither widens nor closes. The resulting order is
pci_rescan_remove_lock, device_lock(any) -> pci_bridge_d3_lock ->
pci_bus_sem (read), so pci_bridge_d3_lock must never be acquired while
holding pci_bus_sem and no pci_walk_bus() callback may call into this
path.

The race dates back to commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into
D3 during suspend"), is theoretical with no known report, and so
carries no Fixes: tag and no stable designation; it claims no measured
performance contribution.

Assisted-by: LLM
Signed-off-by: Pavol Sakac <sakacpav@amazon.de>
---
 drivers/pci/pci.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c62a315c0b4c..b2a159ef125b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3095,17 +3095,36 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
 }
 
 /*
+ * Serializes pci_bridge_d3_update()'s bridge_d3 read-modify-writes and
+ * their upstream propagation.  Ordering: pci_rescan_remove_lock,
+ * device_lock(any) -> pci_bridge_d3_lock -> pci_bus_sem (read); no
+ * pci_walk_bus() callback may call into this path.
+ */
+static DEFINE_MUTEX(pci_bridge_d3_lock);
+
+/**
  * pci_bridge_d3_update - Update bridge D3 capabilities
  * @dev: PCI device which is changed
  *
  * Update upstream bridge PM capabilities accordingly depending on if the
  * device PM configuration was changed or the device is being removed.  The
  * change is also propagated upstream.
+ *
+ * Context: Process context. Takes and releases pci_bridge_d3_lock.
  */
 void pci_bridge_d3_update(struct pci_dev *dev)
 {
 	struct pci_dev *bridge;
 
+	/*
+	 * Unlocked fast path; the loop condition re-evaluates both checks
+	 * under the lock.
+	 */
+	bridge = pci_upstream_bridge(dev);
+	if (!bridge || !pci_bridge_d3_possible(bridge))
+		return;
+
+	mutex_lock(&pci_bridge_d3_lock);
 	while ((bridge = pci_upstream_bridge(dev)) &&
 	       pci_bridge_d3_possible(bridge)) {
 		bool remove = !device_is_registered(&dev->dev);
@@ -3148,6 +3167,7 @@ void pci_bridge_d3_update(struct pci_dev *dev)
 		/* Propagate change to upstream bridges */
 		dev = bridge;
 	}
+	mutex_unlock(&pci_bridge_d3_lock);
 }
 
 /**
@@ -3157,6 +3177,9 @@ void pci_bridge_d3_update(struct pci_dev *dev)
  * This function can be used in drivers to enable D3cold from the device
  * they handle.  It also updates upstream PCI bridge PM capabilities
  * accordingly.
+ *
+ * Context: Process context. Takes and releases pci_bridge_d3_lock;
+ * must not be called from a pci_walk_bus() callback.
  */
 void pci_d3cold_enable(struct pci_dev *dev)
 {
@@ -3174,6 +3197,9 @@ EXPORT_SYMBOL_GPL(pci_d3cold_enable);
  * This function can be used in drivers to disable D3cold from the device
  * they handle.  It also updates upstream PCI bridge PM capabilities
  * accordingly.
+ *
+ * Context: Process context. Takes and releases pci_bridge_d3_lock;
+ * must not be called from a pci_walk_bus() callback.
  */
 void pci_d3cold_disable(struct pci_dev *dev)
 {
-- 
2.47.3