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

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 <[email protected]>
> ---
>  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