Re: [PATCH v5 8/8] PCI: vmd: Add workaround for bus number hardwired to fixed non-zero value

[sashiko-bot@kernel.org]

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

New issues:
- [High] Modifying `bus->number` dynamically after bus registration breaks PCI device naming conventions and sysfs topology consistency.

Pre-existing issues:
- [High] Missing cleanup of VMD_BUS_0 when VMD_BUS_1 creation fails, leading to a PCI root bus leak and a potential Use-After-Free.
--

commit daf784b1aba7feceb67edffc6d0d8a59f8bc2fb9
Author: Szymon Durawa <szymon.durawa@linux.intel.com>

PCI: vmd: Add workaround for bus number hardwired to fixed non-zero value

The VMD BUS1 root bus number is fixed in hardware to 0x80. This patch adds
a workaround to bypass bus number reconfiguration by updating the BUS1
number to be the same as the BUS1 primary number.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index c68ea7837620d..d0527be78ebd5 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -440,8 +440,23 @@ static void vmd_remove_irq_domain(struct vmd_dev *vmd)
>  static void __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
>  				  unsigned int devfn, int reg, int len)
>  {
> -	unsigned int busnr_ecam = bus->number - vmd->busn_start[VMD_BUS_0];
> -	u32 offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
> +	unsigned char bus_number;
> +	unsigned int busnr_ecam;
> +	u32 offset;
> +
> +	/*
> +	 * VMD workaround: for VMD BUS1, bus->number is set to VMD_PRIMARY_BUS1
> +	 * (see comment in vmd_enable_domain() under vmd_create_bus() for BUS1)
> +	 * but original value is 0xE1 which is stored in
> +	 * vmd->busn_start[VMD_BUS_1].
> +	 */
> +	if (vmd->bus1_rootbus && bus->number == VMD_PRIMARY_BUS1)
> +		bus_number = vmd->busn_start[VMD_BUS_1];

[Severity: High]
Does this code undo the bus number mutation performed in vmd_enable_domain()
for config space accesses?

While this ensures config space accesses still work, it highlights the
mismatch that leaves the sysfs topology structurally inconsistent for
userspace.

> +	else
> +		bus_number = bus->number;
> +
> +	busnr_ecam = bus_number - vmd->busn_start[VMD_BUS_0];
> +	offset = PCIE_ECAM_OFFSET(busnr_ecam, devfn, reg);
>  
>  	if (offset + len >= resource_size(&vmd->dev->resource[VMD_CFGBAR]))
>  		return NULL;

[ ... ]

> @@ -1177,6 +1192,19 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  		 */
>  		vmd->bus[VMD_BUS_1]->primary = VMD_PRIMARY_BUS1;

[Severity: High]
This is a pre-existing issue, but does the error handling just above this
code leak VMD_BUS_0?

If vmd_create_bus() for VMD_BUS_1 fails:

vmd_enable_domain() {
    ...
    ret = vmd_create_bus(vmd, VMD_BUS_0, sd, offset);
    if (ret)
        return ret; // Succeeds
    ...
    if (vmd->bus1_rootbus) {
        ret = vmd_create_bus(vmd, VMD_BUS_1, sd, offset);
        if (ret) {
            pci_err(vmd->dev, "Can't create BUS1: %d\n", ret);
            return ret; // Leaks VMD_BUS_0
        }
    ...
}

The error path returns directly without calling pci_stop_root_bus() and
pci_remove_root_bus() for VMD_BUS_0.

Could this lead to a use-after-free if the PCI core retains a reference to
the leaked VMD_BUS_0 after the vmd device allocation is freed?

> +		/*
> +		 * This is a workaround for pci_scan_bridge_extend() code.
> +		 * It marks bridge invalid configuration when detecting a
> +		 * non-zero (0x80) the VMD BUS1 root bus number. Thus Primary Bus Number
> +		 * of Root Ports on BUS1 is deconfigured in the first pass of
> +		 * pci_scan_bridge() to be re-assigned to 0x0 in the second pass.
> +		 * As a result no subordinate bus number behind VMD BUS1 is found.
> +		 * Workaround: update vmd->bus[VMD_BUS_1]->number and
> +		 * vmd->bus[VMD_BUS_1]->primary to the same value, which
> +		 * bypasses bus number reconfiguration.
> +		 */
> +		vmd->bus[VMD_BUS_1]->number = VMD_PRIMARY_BUS1;

[Severity: High]
Does modifying the bus number dynamically after bus registration break PCI
device naming conventions and sysfs topology consistency?

Because the bus number is forcibly mutated after the VMD_BUS_1 root bus
device has already been registered with the PCI subsystem, child devices
downstream will be enumerated and registered using names derived from
0x80 (e.g., 0000:80:1c.0).

Meanwhile, their parent bus directory in sysfs remains named using the
original bus number (e.g., 0000:e1), which can break userspace tools
expecting standard PCI naming.

> +
>  		WARN(sysfs_create_link(&vmd->dev->dev.kobj,
>  				       &vmd->bus[VMD_BUS_1]->dev.kobj,
>  				       "domain1"),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625082940.1290740-1-szymon.durawa@linux.intel.com?part=8