Re: [RFC 10/12] PCI: Add Scalable I/O Virtualization data structure definitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Refactoring of VF routing ID calculation introduces a silent 16-bit truncation that bypasses the maximum bus number bounds check.
--

commit ca9b97e7dff7c499701ce4b55ae9cf14d51bbed7
Author: Dimitri Daskalakis <daskald@meta.com>

PCI: Add Scalable I/O Virtualization data structure definitions

Define the PCIe SIOV extended capability registers per the PCIe 7.0
spec, and introduce the kernel-internal data structures needed to track
SIOV state on a Physical Function. Add an is_siov bit to struct pci_dev
along with helpers to identify SIOV PFs/VFs.

> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 45411960fd2ef..fd7c04e26c164 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -683,6 +683,19 @@ struct pci_sriov {
>  	bool		drivers_autoprobe; /* Auto probing of VFs by driver */
>  };
>  
> +/* Scalable I/O Virtualization */
> +struct pci_siov {
> +	struct pci_dev	*self;		/* This PF */
> +	u32		cap;		/* SIOV Capabilities */
> +	u16		pos;		/* Capability position */
> +	u16		total_SDIs;	/* Total SDIs associated with the PF */
> +	u16		num_SDIs;	/* Number of SDIs currently enabled */
> +	u16		offset;		/* First SDI Routing ID offset */
> +	u16		stride;		/* Following SDI stride */
> +	u16		driver_max_SDIs;/* Max num SDIs driver supports */
> +	u8		max_SDI_buses;	/* Max buses consumed by SDIs */
> +};
> +

[Severity: High]
This patch series relies on the pci_virtfn_routing_id() helper introduced in
an earlier commit ("PCI: Add helper to compute VF Routing ID to pci.h"), but
doesn't this helper introduce a silent truncation issue?

In drivers/pci/pci.h:

static inline u16 pci_virtfn_routing_id(struct pci_dev *pf, u16 offset,
				     u16 stride, int id)
{
	return (pf->bus->number << 8) + pf->devfn + offset + stride * id;
}

If the calculated bus number exceeds 255, the internal calculation exceeds
0xFFFF and is truncated by the u16 return type.

When this is used to determine the bus number in drivers/pci/iov.c:

int pci_iov_virtfn_bus(struct pci_dev *dev, int vf_id)
{
    ...
	return pci_virtfn_routing_id(dev, dev->sriov->offset,
				  dev->sriov->stride, vf_id) >> 8;
}

The right shift is performed on the truncated 16-bit value, resulting in a
wrapped-around bus number.

Consequently, does this allow the bounds check in sriov_enable() to be
bypassed?

drivers/pci/iov.c:sriov_enable() {
    ...
	bus = pci_iov_virtfn_bus(dev, nr_virtfn - 1);
	if (bus > dev->bus->busn_res.end) {
    ...
}

Because the wrapped-around bus value appears to be within bounds, this could
allow VFs to be enabled on invalid, overlapping bus numbers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604150153.3619662-1-dimitri.daskalakis1@gmail.com?part=10