Re: [PATCH v2 02/13] PCI/IOV: Introduce VF iterators

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] PCI/IOV: divide by zero in pci_get_vf() when PF VF stride is zero
--

commit a822ae2dea34f9ae5ccc5c50cffaa927173aa6ac
Author: Lukasz Laguna <lukasz.laguna@intel.com>

PCI/IOV: Introduce VF iterators

This commit introduces pci_get_vf() and pci_get_vf_reverse() along with
for_each_pci_vf(), for_each_pci_vf_reverse(), and
for_each_pci_vf_continue_reverse() macros to iterate over the enabled VFs
of a PF.

> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index f6e74847df72c..da895e7f2683e 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -70,6 +70,69 @@ struct pci_dev *pci_iov_get_virtfn(struct pci_dev *dev, int id)
>  }
>  EXPORT_SYMBOL_GPL(pci_iov_get_virtfn);
>  
> +/**
> + * pci_get_vf() - Return the first or next existing VF of a PF
> + * @dev: PCI PF whose VFs are being iterated
> + * @from: Previous VF returned by this function, or %NULL to return the first VF
> + *
> + * Iterate over existing VFs of @dev in ascending VF ID order, skipping VF IDs
> + * with no device present. If a VF is found, its reference count is incremented.
> + * A new iteration is started by passing %NULL as the @from argument. Otherwise,
> + * iteration continues after @from, and the reference count on @from is
> + * decremented.
> + *
> + * Return: a pointer to the first or next VF, or %NULL if no further VF is found.
> + */
> +struct pci_dev *pci_get_vf(struct pci_dev *dev, struct pci_dev *from)
> +{
> +	int id = 0;
> +
> +	if (from) {
> +		id = pci_iov_vf_id(from) + 1;
> +		pci_dev_put(from);
> +	}
> +
> +	for (; id < pci_num_vf(dev); id++) {
> +		from = pci_iov_get_virtfn(dev, id);
> +		if (from)
> +			return from;
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_get_vf);
> +
> +/**
> + * pci_get_vf_reverse() - Return the last or previous existing VF of a PF
> + * @dev: PCI PF whose VFs are being iterated
> + * @from: Previous VF returned by this function, or %NULL to return the last VF
> + *
> + * Same semantics as pci_get_vf(), except iteration proceeds in descending VF
> + * ID order.
> + *
> + * Return: a pointer to the last or previous VF, or %NULL if no further VF is
> + * found.
> + */
> +struct pci_dev *pci_get_vf_reverse(struct pci_dev *dev, struct pci_dev *from)
> +{
> +	int id = pci_num_vf(dev);
> +
> +	if (from) {
> +		id = pci_iov_vf_id(from);
> +		pci_dev_put(from);
> +	}

[Severity: High]
Can pci_get_vf() and pci_get_vf_reverse() trigger a divide-by-zero when
iterating over a PF whose SR-IOV VF stride (pf->sriov->stride) is 0?

In drivers/pci/iov.c, compute_max_vf_buses() validates offset and stride,
explicitly permitting iov->stride == 0 when nr_virtfn == 1 (per commit
ea9a8854161d, "allow numVF==1 && stride==0"):

drivers/pci/iov.c:compute_max_vf_buses() {
    ...
	for (nr_virtfn = iov->total_VFs; nr_virtfn; nr_virtfn--) {
		pci_iov_set_numvfs(dev, nr_virtfn);
		if (!iov->offset || (nr_virtfn > 1 && !iov->stride)) {
			rc = -EIO;
			goto out;
		}
    ...
}

When SR-IOV is enabled with nr_virtfn == 1 on a device reporting stride == 0,
sriov_enable() -> pci_iov_add_virtfn(dev, 0) succeeds because
pci_iov_virtfn_bus(dev, 0) and pci_iov_virtfn_devfn(dev, 0) multiply
dev->sriov->stride by 0, creating VF 0 with pf->sriov->stride == 0.

When iterating via for_each_pci_vf() or for_each_pci_vf_reverse(), the first
call with from == NULL looks up VF 0 via pci_iov_get_virtfn(dev, 0) and
returns it. On the subsequent step to advance past VF 0 and terminate the
loop, pci_get_vf() and pci_get_vf_reverse() call pci_iov_vf_id(from), which
divides unconditionally by pf->sriov->stride:

drivers/pci/iov.c:pci_iov_vf_id() {
    ...
	pf = pci_physfn(dev);
	return (pci_dev_id(dev) - (pci_dev_id(pf) + pf->sriov->offset)) /
	       pf->sriov->stride;
}

A similar division by zero can also occur if NumVFs was reset to 0 via
pci_iov_set_numvfs(dev, 0), which re-reads PCI_SRIOV_VF_STRIDE while
NumVFs == 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027160446.858300-1-lukasz.laguna@intel.com?part=2