linux-kernel - Re: [PATCH 2/2] PCI: Add lockdep assertion in pci_stop_and_remove_bus

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: <ac80630fda0c85ee1194ab08974aaf5f93e09cf9.camel@linux.ibm.com>
Date: Thu, 25 Sep 2025 09:25:10 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Farhan Ali <alifm@...ux.ibm.com>, Bjorn Helgaas <helgaas@...nel.org>,
        Lukas Wunner <lukas@...ner.de>
Cc: Keith Busch <kbusch@...nel.org>, Gerd Bayer <gbayer@...ux.ibm.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Benjamin Block
 <bblock@...ux.ibm.com>,
        Halil Pasic <pasic@...ux.ibm.com>,
        Julian Ruess
 <julianr@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik
 <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] PCI: Add lockdep assertion in
 pci_stop_and_remove_bus_device()

On Wed, 2025-09-24 at 11:06 -0700, Farhan Ali wrote:
> On 8/26/2025 1:52 AM, Niklas Schnelle wrote:
> > Removing a PCI devices requires holding pci_rescan_remove_lock. Prompted
> > by this being missed in sriov_disable() and going unnoticed since its
> > inception add a lockdep assert so this doesn't get missed again in the
> > future.
> > 
> > Reviewed-by: Benjamin Block <bblock@...ux.ibm.com>
> > Signed-off-by: Niklas Schnelle <schnelle@...ux.ibm.com>
> > ---
> >   drivers/pci/pci.h    | 2 ++
> >   drivers/pci/probe.c  | 2 +-
> >   drivers/pci/remove.c | 1 +
> >   3 files changed, 4 insertions(+), 1 deletion(-)
> > 
--- snip ---
> > diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> > index 445afdfa6498edc88f1ef89df279af1419025495..0b9a609392cecba36a818bc496a0af64061c259a 100644
> > --- a/drivers/pci/remove.c
> > +++ b/drivers/pci/remove.c
> > @@ -138,6 +138,7 @@ static void pci_remove_bus_device(struct pci_dev *dev)
> >    */
> >   void pci_stop_and_remove_bus_device(struct pci_dev *dev)
> >   {
> > +	lockdep_assert_held(&pci_rescan_remove_lock);
> >   	pci_stop_bus_device(dev);
> >   	pci_remove_bus_device(dev);
> >   }
> 
> We also have the function pci_stop_and_remove_bus_device_locked() as 
> Gerd mentioned, so is pci_stop_and_remove_bus_device meant to be called 
> without the rescan_remove_lock held? This is a little confusing as we 
> shouldn't be adding/removing from the bus without the lock AFAIU, but 
> maybe I am missing something?
> 
> Thanks
> Farhan

As far as I understand one would use pci_stop_and_remove_bus_device()
if one is already holding the pci_rescan_remove_lock and
pci_stop_and_remove_bus_device_locked() if one only needs to take it
for that call. I think this is kind of easy to get confused about so
this lockdep assertion is even more useful.