Re: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Acquiring `device_lock()` inside `vfio_pci_core_close_device()` introduces a guaranteed AB-BA deadlock if the device is unbound from the driver while still in use by userspace.
- [High] Synchronously tearing down SR-IOV VFs from the PF's `close_device` path deadlocks if any VF is actively in use by VFIO.
- [High] Calling `pci_disable_sriov()` without holding `pci_rescan_remove_lock` causes an immediate lockdep splat and races with PCI bus enumeration.
- [Medium] Time-of-Check to Time-of-Use (TOCTOU) races around `device_lock()` allow VFs to be enabled immediately before hardware reset, defeating the purpose of the patch.
--

--- Patch [1]: [RFC PATCH 1/1] vfio/pci: Disable sriov on PF device close ---
commit 996eca93815cca91725d215b78730f993e057c49
Author: Samiullah Khawaja <skhawaja@google.com>

vfio/pci: Disable sriov on PF device close

When a user closes a VFIO device file descriptor for an SR-IOV Physical
Function, the device is reset. This reset disrupts the VFs without notifying
the core PCI driver model. This commit disables SR-IOV prior to issuing the
PF reset so VFs are torn down gracefully at the software level.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index a113c55845e10..b36c77eb3c407 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -826,6 +826,13 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
>  #if IS_ENABLED(CONFIG_EEH)
>  	eeh_dev_release(vdev->pdev);
>  #endif
> +
> +	if (pci_num_vf(vdev->pdev)) {

[Severity: Medium]
Is it possible for VFs to be enabled concurrently right after this check?

Since pci_num_vf() is evaluated locklessly before acquiring the device lock, a
concurrent sysfs write to sriov_numvfs could enable VFs immediately after this
returns 0, causing the teardown to be skipped.

Additionally, because the device lock is dropped before the hardware reset in
vfio_pci_core_disable(), a concurrent sysfs write can acquire the lock,
successfully enable SR-IOV, and release the lock before the reset occurs.

Could this recreate the exact hardware state mismatch the patch attempts to fix?

> +		device_lock(&vdev->pdev->dev);

[Severity: High]
Does acquiring the device lock here introduce an AB-BA deadlock during device
unbind?

When a device is unbound, the PCI core acquires the device lock in
device_release_driver() and invokes the driver's remove callback. For
vfio-pci, this calls vfio_unregister_group_dev(), which blocks indefinitely
waiting for userspace to close all active file descriptors.

When userspace closes the device fd, vfio_pci_core_close_device() is invoked.
Because this patch tries to acquire the device lock, the fd release thread
will block forever waiting for the unbind thread, and the unbind thread will
wait forever for the fd release thread to finish.

> +		vfio_pci_core_sriov_configure(vdev, 0);

[Severity: High]
Can tearing down VFs from the PF's close path deadlock if a VF is actively in
use by VFIO?

When vfio_pci_core_close_device() calls pci_disable_sriov(), the PCI core
synchronously removes all VF devices from the bus. If a VF is also bound to
vfio-pci and currently opened by userspace, removing the VF triggers
vfio_pci_remove(), which blocks indefinitely until the VF file descriptor is
closed.

If a single process holds both the PF and VF open and exits, exit_files()
schedules the fput() for both file descriptors sequentially. If the PF is
closed first, the thread hangs here waiting for the VF to close, preventing
the VF's fput() from ever running.

[Severity: High]
Does this code trigger a lockdep warning by manipulating the PCI bus topology
without the required lock?

Calling vfio_pci_core_sriov_configure() here ultimately calls
pci_stop_and_remove_bus_device() to remove VFs from the PCI bus. That function
enforces a strict locking requirement via
lockdep_assert_held(&pci_rescan_remove_lock).

Because this global mutex is omitted, closing a PF with active VFs could race
against concurrent PCI hotplug or sysfs rescan operations.

> +		device_unlock(&vdev->pdev->dev);
> +	}
> +
>  	vfio_pci_dma_buf_cleanup(vdev);
>  
>  	vfio_pci_core_disable(vdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805003355.728299-2-skhawaja@google.com?part=1