Re: [PATCH] fix: PCI: pci_iov_remove_virtfn: fix unmatched pci_dev_put for PF device

[sashiko-bot@kernel.org]

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

New issues:
- [High] Refcount Leak of PF device

Pre-existing issues:
- [High] Premature Release / Use-After-Free of PF device in VF teardown
--

commit f1226b4e04690771e292377b3796f15f2ab401f4
Author: WenTao Liang <vulab@iscas.ac.cn>

fix: PCI: pci_iov_remove_virtfn: fix unmatched pci_dev_put for PF device

This patch removes a pci_dev_put() call for the PF device in
pci_iov_remove_virtfn(). The commit message states that the reference was never
acquired by the function, potentially causing a refcount underflow and
premature release.

> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index b0d24839c0840..87a2f3b278683 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -426,7 +426,6 @@ void pci_iov_remove_virtfn(struct pci_dev *dev, int id)
>  
>  	/* balance pci_get_domain_bus_and_slot() */
>  	pci_dev_put(virtfn);
> -	pci_dev_put(dev);

[Severity: High]
Does removing this call cause a refcount leak for the PF device?

Looking at the VF creation in pci_iov_scan_device(), it appears a reference
to the PF is explicitly acquired:

drivers/pci/iov.c:pci_iov_scan_device() {
    ...
    virtfn->physfn = pci_dev_get(dev);
    ...
}

Because the core release function for the VF (pci_release_dev()) doesn't
currently drop this reference, doesn't removing this cleanup result in a
persistent leak of the PF device every time SR-IOV is disabled?

[Severity: High]
This is a pre-existing issue, but there appears to be a lifecycle design flaw
with when this PF reference is dropped.

If the VF object outlives pci_iov_remove_virtfn() due to elevated references
(like an open sysfs file or concurrent VFIO access pinning the VF's kobject),
does dropping the PF reference here leave a dangling virtfn->physfn pointer?

If the PF is subsequently removed and freed, any later access to virtfn->physfn
by the surviving VF object could trigger a use-after-free.

Should the balancing pci_dev_put(pci_dev->physfn) instead be moved into the
VF's actual release handler to ensure the PF is kept alive exactly as long as
the VF memory object?

drivers/pci/probe.c:pci_release_dev() {
    ...
    if (pci_dev->is_virtfn)
        pci_dev_put(pci_dev->physfn);
    ...
}

>  }
>  
>  static ssize_t sriov_totalvfs_show(struct device *dev,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626155042.53862-1-vulab@iscas.ac.cn?part=1