[PATCH] PCI: Fix use-after-free race in pci_find_bus()

[PATCH] PCI: Fix use-after-free race in pci_find_bus()

[Mohamad Raizudeen]

pci_find_bus() iterates over the list of PCI root buses using
pci_find_next_bus(). This helper acquires pci_bus_sem, retrieves the
next bus and drops the lock before returning the pointer to the caller.

pci_find_bus() then uses this pointer to check the domain and traverses
the child buses via pci_do_find_bus() without holding the pci_bus_sem
lock.

If a PCI bus is concurrently removed for example via hotplug between
loop iterations, the from pointer passed back into pci_find_next_bus()
becomes stale, leading to a user-after-free when dereferencing
from->node.next. Additionally, traversing the bus tree without holding
the lock is a race condition.

Fix this by iterating pci_root_buses list directly using
list_for_each_entry() inside pci_find_bus() while holding the
pci_bus_sem read lock for the entire duration of the search. This
ensures the list and tree structures cannot change while being
traversed, eliminating the use-after-free.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
 drivers/pci/search.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index e3d3177fce54..f50e83061b76 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -142,17 +142,19 @@ static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
  */
 struct pci_bus *pci_find_bus(int domain, int busnr)
 {
-	struct pci_bus *bus = NULL;
-	struct pci_bus *tmp_bus;
+	struct pci_bus *bus;
+	struct pci_bus *tmp_bus = NULL;
 
-	while ((bus = pci_find_next_bus(bus)) != NULL)  {
-		if (pci_domain_nr(bus) != domain)
-			continue;
-		tmp_bus = pci_do_find_bus(bus, busnr);
-		if (tmp_bus)
-			return tmp_bus;
+	down_read(&pci_bus_sem);
+	list_for_each_entry(bus, &pci_root_buses, node) {
+		if (pci_domain_nr(bus) == domain) {
+			tmp_bus = pci_do_find_bus(bus, busnr);
+			if (tmp_bus)
+				break;
+		}
 	}
-	return NULL;
+	up_read(&pci_bus_sem);
+	return tmp_bus;
 }
 EXPORT_SYMBOL(pci_find_bus);
 
-- 
2.53.0

Re: [PATCH] PCI: Fix use-after-free race in pci_find_bus()

[Greg KH]

On Wed, Aug 12, 2026 at 08:17:13AM +0530, Mohamad Raizudeen wrote:
> pci_find_bus() iterates over the list of PCI root buses using
> pci_find_next_bus(). This helper acquires pci_bus_sem, retrieves the
> next bus and drops the lock before returning the pointer to the caller.
> 
> pci_find_bus() then uses this pointer to check the domain and traverses
> the child buses via pci_do_find_bus() without holding the pci_bus_sem
> lock.
> 
> If a PCI bus is concurrently removed for example via hotplug between
> loop iterations, the from pointer passed back into pci_find_next_bus()
> becomes stale, leading to a user-after-free when dereferencing
> from->node.next. Additionally, traversing the bus tree without holding
> the lock is a race condition.

Did you find this actually happens?  How did you find this at all?

> 
> Fix this by iterating pci_root_buses list directly using
> list_for_each_entry() inside pci_find_bus() while holding the
> pci_bus_sem read lock for the entire duration of the search. This
> ensures the list and tree structures cannot change while being
> traversed, eliminating the use-after-free.

Why do two changes here, and not just make a patch series?

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> ---
>  drivers/pci/search.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index e3d3177fce54..f50e83061b76 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -142,17 +142,19 @@ static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
>   */
>  struct pci_bus *pci_find_bus(int domain, int busnr)
>  {
> -	struct pci_bus *bus = NULL;
> -	struct pci_bus *tmp_bus;
> +	struct pci_bus *bus;
> +	struct pci_bus *tmp_bus = NULL;
>  
> -	while ((bus = pci_find_next_bus(bus)) != NULL)  {
> -		if (pci_domain_nr(bus) != domain)
> -			continue;
> -		tmp_bus = pci_do_find_bus(bus, busnr);
> -		if (tmp_bus)
> -			return tmp_bus;
> +	down_read(&pci_bus_sem);
> +	list_for_each_entry(bus, &pci_root_buses, node) {
> +		if (pci_domain_nr(bus) == domain) {
> +			tmp_bus = pci_do_find_bus(bus, busnr);
> +			if (tmp_bus)
> +				break;
> +		}

Are you sure this logic is the same as the original?
pci_find_next_bus() does grab the needed lock here, so why do you think
this is racy?

And pci_bus_sem is just for root busses, not the individual busses,
right?

How was this tested?

>  	}
> -	return NULL;
> +	up_read(&pci_bus_sem);

Why not use guard() instead?

thanks,

greg k-h

Re: [PATCH] PCI: Fix use-after-free race in pci_find_bus()

[Mohamad Raizudeen]

On Wed, Aug 12, 2026 at 12:00:33PM +0900, Greg KH wrote:
> On Wed, Aug 12, 2026 at 08:17:13AM +0530, Mohamad Raizudeen wrote:
> > pci_find_bus() iterates over the list of PCI root buses using
> > pci_find_next_bus(). This helper acquires pci_bus_sem, retrieves the
> > next bus and drops the lock before returning the pointer to the caller.
> > 
> > pci_find_bus() then uses this pointer to check the domain and traverses
> > the child buses via pci_do_find_bus() without holding the pci_bus_sem
> > lock.
> > 
> > If a PCI bus is concurrently removed for example via hotplug between
> > loop iterations, the from pointer passed back into pci_find_next_bus()
> > becomes stale, leading to a user-after-free when dereferencing
> > from->node.next. Additionally, traversing the bus tree without holding
> > the lock is a race condition.
> 
> Did you find this actually happens?  How did you find this at all?

I found this purely through by reading and reviewing the code, while
analyzing the locking patterns in the PCI subsystem. I have not seen it
crash in production, but the race condition is statically clear from
reading the code.
>
> > 
> > Fix this by iterating pci_root_buses list directly using
> > list_for_each_entry() inside pci_find_bus() while holding the
> > pci_bus_sem read lock for the entire duration of the search. This
> > ensures the list and tree structures cannot change while being
> > traversed, eliminating the use-after-free.
> 
> Why do two changes here, and not just make a patch series?
I did both in one patch because they are connected. Since
pci_find_next_bus() drops the lock early, I couldn't use it to hold the
lock for the whole search. I had to change the loop to fix the locking.
> 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> > ---
> >  drivers/pci/search.c | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> > index e3d3177fce54..f50e83061b76 100644
> > --- a/drivers/pci/search.c
> > +++ b/drivers/pci/search.c
> > @@ -142,17 +142,19 @@ static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
> >   */
> >  struct pci_bus *pci_find_bus(int domain, int busnr)
> >  {
> > -	struct pci_bus *bus = NULL;
> > -	struct pci_bus *tmp_bus;
> > +	struct pci_bus *bus;
> > +	struct pci_bus *tmp_bus = NULL;
> >  
> > -	while ((bus = pci_find_next_bus(bus)) != NULL)  {
> > -		if (pci_domain_nr(bus) != domain)
> > -			continue;
> > -		tmp_bus = pci_do_find_bus(bus, busnr);
> > -		if (tmp_bus)
> > -			return tmp_bus;
> > +	down_read(&pci_bus_sem);
> > +	list_for_each_entry(bus, &pci_root_buses, node) {
> > +		if (pci_domain_nr(bus) == domain) {
> > +			tmp_bus = pci_do_find_bus(bus, busnr);
> > +			if (tmp_bus)
> > +				break;
> > +		}
> 
> Are you sure this logic is the same as the original?
Yes, I used list_for_each_entry() that does the exact same thing as the
old while loop, but it let me keep the lock saefely for the whole
search.

> pci_find_next_bus() does grab the needed lock here, so why do you think
> this is racy?
You are right, it grabs the lock. But it drops the lock before returning
the bus pointer. So the caller then uses that pointer without a lock and
passes it back for the next loop. If a bus is removed in that moment,
the next call reads freed memory.
> 
> And pci_bus_sem is just for root busses, not the individual busses,
> right?
It protects the root bus list, but it also protects the child buses.
Since pci_do_find_bus() walks through the child buses, needed to hold
the lock to make that safe too.
> 
> How was this tested?
I compiled ir and booted it in x86_64 qemu vm. It booted fine without
any PCI crashes. I will run the same test on v2 patch before sending it.
> 
> >  	}
> > -	return NULL;
> > +	up_read(&pci_bus_sem);
> 
> Why not use guard() instead?
Honestly, I was so focused on getting the locking logic right that I
completely missed I meant forgot to use guard(). I will make sure to use
guard() in the v2 patch.
> 
> thanks,
> 
> greg k-h

Thanks & regards,
Mohamad Raizudeen 

Re: [PATCH] PCI: Fix use-after-free race in pci_find_bus()

[Greg KH]

On Wed, Aug 12, 2026 at 12:56:59PM +0530, Mohamad Raizudeen wrote:
> On Wed, Aug 12, 2026 at 12:00:33PM +0900, Greg KH wrote:
> > On Wed, Aug 12, 2026 at 08:17:13AM +0530, Mohamad Raizudeen wrote:
> > > pci_find_bus() iterates over the list of PCI root buses using
> > > pci_find_next_bus(). This helper acquires pci_bus_sem, retrieves the
> > > next bus and drops the lock before returning the pointer to the caller.
> > > 
> > > pci_find_bus() then uses this pointer to check the domain and traverses
> > > the child buses via pci_do_find_bus() without holding the pci_bus_sem
> > > lock.
> > > 
> > > If a PCI bus is concurrently removed for example via hotplug between
> > > loop iterations, the from pointer passed back into pci_find_next_bus()
> > > becomes stale, leading to a user-after-free when dereferencing
> > > from->node.next. Additionally, traversing the bus tree without holding
> > > the lock is a race condition.
> > 
> > Did you find this actually happens?  How did you find this at all?
> 
> I found this purely through by reading and reviewing the code, while
> analyzing the locking patterns in the PCI subsystem. I have not seen it
> crash in production, but the race condition is statically clear from
> reading the code.
> >
> > > 
> > > Fix this by iterating pci_root_buses list directly using
> > > list_for_each_entry() inside pci_find_bus() while holding the
> > > pci_bus_sem read lock for the entire duration of the search. This
> > > ensures the list and tree structures cannot change while being
> > > traversed, eliminating the use-after-free.
> > 
> > Why do two changes here, and not just make a patch series?
> I did both in one patch because they are connected. Since
> pci_find_next_bus() drops the lock early, I couldn't use it to hold the
> lock for the whole search. I had to change the loop to fix the locking.
> > 
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> > > ---
> > >  drivers/pci/search.c | 20 +++++++++++---------
> > >  1 file changed, 11 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> > > index e3d3177fce54..f50e83061b76 100644
> > > --- a/drivers/pci/search.c
> > > +++ b/drivers/pci/search.c
> > > @@ -142,17 +142,19 @@ static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
> > >   */
> > >  struct pci_bus *pci_find_bus(int domain, int busnr)
> > >  {
> > > -	struct pci_bus *bus = NULL;
> > > -	struct pci_bus *tmp_bus;
> > > +	struct pci_bus *bus;
> > > +	struct pci_bus *tmp_bus = NULL;
> > >  
> > > -	while ((bus = pci_find_next_bus(bus)) != NULL)  {
> > > -		if (pci_domain_nr(bus) != domain)
> > > -			continue;
> > > -		tmp_bus = pci_do_find_bus(bus, busnr);
> > > -		if (tmp_bus)
> > > -			return tmp_bus;
> > > +	down_read(&pci_bus_sem);
> > > +	list_for_each_entry(bus, &pci_root_buses, node) {
> > > +		if (pci_domain_nr(bus) == domain) {
> > > +			tmp_bus = pci_do_find_bus(bus, busnr);
> > > +			if (tmp_bus)
> > > +				break;
> > > +		}
> > 
> > Are you sure this logic is the same as the original?
> Yes, I used list_for_each_entry() that does the exact same thing as the
> old while loop, but it let me keep the lock saefely for the whole
> search.
> 
> > pci_find_next_bus() does grab the needed lock here, so why do you think
> > this is racy?
> You are right, it grabs the lock. But it drops the lock before returning
> the bus pointer. So the caller then uses that pointer without a lock and
> passes it back for the next loop. If a bus is removed in that moment,
> the next call reads freed memory.
> > 
> > And pci_bus_sem is just for root busses, not the individual busses,
> > right?
> It protects the root bus list, but it also protects the child buses.
> Since pci_do_find_bus() walks through the child buses, needed to hold
> the lock to make that safe too.
> > 
> > How was this tested?
> I compiled ir and booted it in x86_64 qemu vm. It booted fine without
> any PCI crashes. I will run the same test on v2 patch before sending it.

Booting in a vm is very simple as a vm does not have many PCI devices.
Try it on a real system with a big topology as well as a pci hotplug
system please.

Also note that this function should only be called when pci devices are
being added to the system, so odds are it can't race with a device being
removed due to the pci bus lock in the first place, right?

thanks,

greg k-h