Re: [PATCH] PCI: Keep an already-configured bridge's bus number across a rescan

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: bus number overlap and integer underflow in used_buses on rescan
- [High] pci: bus number leak and integer overflow truncating bus to 0
--

commit 327bbd815883ab87700e0adf3427c36a03e138ab
Author: Jianjun Wang <jianjun.wang@linux.alibaba.com>

PCI: Keep an already-configured bridge's bus number across a rescan

This commit changes pci_scan_bridge_extend() to reuse an existing child bus
number across rescans instead of assigning max + 1. This prevents bridges
from taking over a sibling's child bus and swapping subtrees when they are
re-discovered out of bus-number order.

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..6b418d20cde34 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1513,6 +1513,18 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  		fixed_buses = pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
>  		if (fixed_buses)
>  			next_busnr = fixed_sec;
> +		else if (dev->subordinate)

[Severity: High]
Does preserving the existing bus number here cause a regression where bus
numbers permanently leak during repeated hot-removes and rescans?

If preserved bridges are never renumbered down to close gaps, and new
bridges are always assigned max + 1, max will only go up and eventually
exceed 255. When next_busnr hits 256, does pci_find_bus() implicitly
truncate it to 0 (due to pci_do_find_bus()'s unsigned char argument), finding
the root bus instead?

If child evaluates to the root bus, it could misconfigure the new bridge
with root bus parameters later in pci_scan_bridge_extend():

	buses = (buses & PCI_SEC_LATENCY_TIMER_MASK) |
		FIELD_PREP(PCI_PRIMARY_BUS_MASK, child->primary) |
		FIELD_PREP(PCI_SECONDARY_BUS_MASK, child->busn_res.start) |
		FIELD_PREP(PCI_SUBORDINATE_BUS_MASK, child->busn_res.end);

> +			/*
> +			 * This bridge already has a child bus, so we are
> +			 * rescanning an already-configured bridge (e.g. after a
> +			 * sibling bridge was removed and the bus was rescanned).
> +			 * Keep its existing bus number instead of renumbering
> +			 * it: a re-added sibling is appended to the tail of the
> +			 * device list, so handing out max + 1 here would let that
> +			 * later sibling steal this bridge's number and swap the
> +			 * two subtrees.
> +			 */
> +			next_busnr = dev->subordinate->busn_res.start;

[Severity: High]
If a hotplug event expands a preceding bridge's subordinate range during
rescan, the global max bus number increases. Does this unconditionally reuse
the old subordinate->busn_res.start as next_busnr, which may now be less
than max?

This regression would cause the local max returned by
pci_scan_bridge_extend() to shrink. In the caller
pci_scan_child_bus_extend(), can max - cmax then severely underflow?

	if (max - cmax > 1)
		used_buses += max - cmax - 1;

This underflow could break hotplug bus distribution and corrupt the PCI
topology.

>  		else
>  			next_busnr = max + 1;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924054115.75574-1-jianjun.wang@linux.alibaba.com?part=1