linux-kernel - Re: [RESEND PATCH v2] PCI: Fix incorrect unlocking in pci_slot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <697a79af45154_3095100b4@dwillia2-mobl4.notmuch>
Date: Wed, 28 Jan 2026 13:03:43 -0800
From: <dan.j.williams@...el.com>
To: Jinhui Guo <guojinhui.liam@...edance.com>, <bhelgaas@...gle.com>,
	<dan.j.williams@...el.com>, <dave.jiang@...el.com>,
	<ilpo.jarvinen@...ux.intel.com>, <kbusch@...nel.org>
CC: <guojinhui.liam@...edance.com>, <linux-kernel@...r.kernel.org>,
	<linux-pci@...r.kernel.org>, <stable@...r.kernel.org>
Subject: Re: [RESEND PATCH v2] PCI: Fix incorrect unlocking in
 pci_slot_trylock()

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:
> 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@...r.kernel.org
> Signed-off-by: Jinhui Guo <guojinhui.liam@...edance.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.com/
> 
> 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

I ended up also reviewing Keith's version of the same [1], but since this
one was posted earlier, go with this one.

Reviewed-by: Dan Williams <dan.j.williams@...el.com>

[1]: http://lore.kernel.org/20260116184150.3013258-1-kbusch@meta.com