Commit a4e772898f8b ("PCI: Add missing bridge lock to pci_bus_lock()") delegates the bridge device's pci_dev_trylock() to pci_bus_trylock() in pci_slot_trylock(), but it forgets to remove the corresponding pci_dev_unlock() when pci_bus_trylock() fails.
Before the commit, the code did:
if (!pci_dev_trylock(dev)) /* <- lock bridge device */ goto unlock; if (dev->subordinate) { if (!pci_bus_trylock(dev->subordinate)) { pci_dev_unlock(dev); /* <- unlock bridge device */ goto unlock; } }
After the commit the bridge-device lock is no longer taken, but the pci_dev_unlock(dev) on the failure path was left in place, leading to the bug.
This yields one of two errors: 1. A warning that the lock is being unlocked when no one holds it. 2. An incorrect unlock of a lock that belongs to another thread.
Fix it by removing the now-redundant pci_dev_unlock(dev) on the failure path.
Fixes: a4e772898f8b ("PCI: Add missing bridge lock to pci_bus_lock()") Cc: stable@vger.kernel.org Signed-off-by: Jinhui Guo guojinhui.liam@bytedance.com Acked-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com ---
Hi, all
v1: https://lore.kernel.org/all/20251211123635.2215-1-guojinhui.liam@bytedance.c...
Changelog in v1 -> v2 - The v1 commit message was too brief, so I’ve sent v2 with more detail. - Remove the braces from the if (!pci_bus_trylock(dev->subordinate)) statement.
Sorry for the noise.
Best Regards, Jinhui
drivers/pci/pci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 13dbb405dc31..59319e08fca6 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5346,10 +5346,8 @@ static int pci_slot_trylock(struct pci_slot *slot) if (!dev->slot || dev->slot != slot) continue; if (dev->subordinate) { - if (!pci_bus_trylock(dev->subordinate)) { - pci_dev_unlock(dev); + if (!pci_bus_trylock(dev->subordinate)) goto unlock; - } } else if (!pci_dev_trylock(dev)) goto unlock; }
On Fri, 12 Dec 2025, Jinhui Guo wrote:
Commit a4e772898f8b ("PCI: Add missing bridge lock to pci_bus_lock()") delegates the bridge device's pci_dev_trylock() to pci_bus_trylock() in pci_slot_trylock(), but it forgets to remove the corresponding pci_dev_unlock() when pci_bus_trylock() fails.
Before the commit, the code did:
if (!pci_dev_trylock(dev)) /* <- lock bridge device */ goto unlock; if (dev->subordinate) { if (!pci_bus_trylock(dev->subordinate)) { pci_dev_unlock(dev); /* <- unlock bridge device */ goto unlock; } }
After the commit the bridge-device lock is no longer taken, but the pci_dev_unlock(dev) on the failure path was left in place, leading to the bug.
This yields one of two errors:
- A warning that the lock is being unlocked when no one holds it.
- An incorrect unlock of a lock that belongs to another thread.
Fix it by removing the now-redundant pci_dev_unlock(dev) on the failure path.
Fixes: a4e772898f8b ("PCI: Add missing bridge lock to pci_bus_lock()") Cc: stable@vger.kernel.org Signed-off-by: Jinhui Guo guojinhui.liam@bytedance.com Acked-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
Please don't make tags like this unless the other people explicitly give them to the patch.
Other than that, this looks okay to me.
Commit a4e772898f8b ("PCI: Add missing bridge lock to pci_bus_lock()") delegates the bridge device's pci_dev_trylock() to pci_bus_trylock() in pci_slot_trylock(), but it forgets to remove the corresponding pci_dev_unlock() when pci_bus_trylock() fails.
Before the commit, the code did:
if (!pci_dev_trylock(dev)) /* <- lock bridge device */ goto unlock; if (dev->subordinate) { if (!pci_bus_trylock(dev->subordinate)) { pci_dev_unlock(dev); /* <- unlock bridge device */ goto unlock; } }
After the commit the bridge-device lock is no longer taken, but the pci_dev_unlock(dev) on the failure path was left in place, leading to the bug.
This yields one of two errors: 1. A warning that the lock is being unlocked when no one holds it. 2. An incorrect unlock of a lock that belongs to another thread.
Fix it by removing the now-redundant pci_dev_unlock(dev) on the failure path.
Fixes: a4e772898f8b ("PCI: Add missing bridge lock to pci_bus_lock()") Cc: stable@vger.kernel.org Signed-off-by: Jinhui Guo guojinhui.liam@bytedance.com ---
Hi, all
Resent v2 to drop the Acked-by tag; no code changes. Sorry for the noise again.
v1: https://lore.kernel.org/all/20251211123635.2215-1-guojinhui.liam@bytedance.c...
Changelog in v1 -> v2 - The v1 commit message was too brief, so I’ve sent v2 with more detail. - Remove the braces from the if (!pci_bus_trylock(dev->subordinate)) statement.
Best Regards, Jinhui
drivers/pci/pci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 13dbb405dc31..59319e08fca6 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5346,10 +5346,8 @@ static int pci_slot_trylock(struct pci_slot *slot) if (!dev->slot || dev->slot != slot) continue; if (dev->subordinate) { - if (!pci_bus_trylock(dev->subordinate)) { - pci_dev_unlock(dev); + if (!pci_bus_trylock(dev->subordinate)) goto unlock; - } } else if (!pci_dev_trylock(dev)) goto unlock; }
linux-stable-mirror@lists.linaro.org