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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The fundamentally vulnerable pci_find_next_bus() iterator is left unmodified, retaining a use-after-free race.
- [High] pci_find_bus() returns a non-refcounted bus pointer after dropping pci_bus_sem, exposing callers to a caller-side use-after-free.
--

--- Patch [1]: [PATCH] PCI: Fix use-after-free race in pci_find_bus() ---
commit 7750909a991524c7f4e93f423648abcd167d3923
Author: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>

PCI: Fix use-after-free race in pci_find_bus()
    
pci_find_bus() iterates over the list of PCI root buses using
pci_find_next_bus(). If a PCI bus is concurrently removed via hotplug
between loop iterations, the from pointer passed back into
pci_find_next_bus() becomes stale. This commit fixes this by iterating
pci_root_buses list directly while holding the pci_bus_sem read lock.

> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index e3d3177fce549..f50e83061b769 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)  {

[Severity: High]
This is a pre-existing issue, but while you are removing the use of
pci_find_next_bus() here, the pci_find_next_bus() function itself remains
vulnerable to the same use-after-free race.

Other lockless callers like uncore_die_to_segment() in
arch/x86/events/intel/uncore.c still iterate over buses using
pci_find_next_bus() without holding pci_lock_rescan_remove(). Because
pci_find_next_bus() drops pci_bus_sem before returning, a concurrent PCI
hot-remove can free the bus. 

When the caller passes the freed bus back into pci_find_next_bus() for the
next iteration, from->node.next is dereferenced, causing a use-after-free.

Should pci_find_next_bus() also be fixed to prevent memory corruption for
other callers?

> -		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;

[Severity: High]
This is a pre-existing issue, but does this expose the caller to a
use-after-free race?

By dropping pci_bus_sem before returning tmp_bus, the returned bus pointer
is un-refcounted and no longer protected by the lock. If the caller does
not hold pci_lock_rescan_remove(), a concurrent hot-remove thread could
immediately acquire pci_bus_sem for write, remove the bus, and free its
memory before the caller has a chance to safely use it.

Would it be safer to increment the reference count of the returned bus
with pci_bus_get() before releasing pci_bus_sem?

>  }
>  EXPORT_SYMBOL(pci_find_bus);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812024713.4958-1-raizudeen.kerneldev@gmail.com?part=1