Re: [RFC PATCH] vfio/pci: Block for the upstream bridge lock in vfio_pci_core_disable()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing pci_dev_trylock(bridge) with a blocking pci_dev_lock(bridge) introduces a 3-thread AB-BA-CA deadlock.
--

commit 170de39cb1fd30df6e523a31c390c9881837b836
Author: YuanShang <YuanShang.Mao@amd.com>

vfio/pci: Block for the upstream bridge lock in vfio_pci_core_disable()

This commit changes vfio_pci_core_disable() to take the upstream bridge lock
blocking instead of with a trylock. This is intended to serialize concurrent
resets of virtual functions under the same bridge.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 6757054e9d875..01f0238057c76 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -791,8 +791,8 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
>  	 */
>  	if (vdev->reset_works) {
>  		bridge = pci_upstream_bridge(pdev);
> -		if (bridge && !pci_dev_trylock(bridge))
> -			goto out_restore_state;
> +		if (bridge)
> +			pci_dev_lock(bridge);

[Severity: High]
Does this blocking lock create a deadlock cycle with PCI device removal and
bus reset operations?

Replacing pci_dev_trylock(bridge) with a blocking pci_dev_lock(bridge)
introduces a 3-thread AB-BA-CA deadlock. This can freeze the involved threads
and lock the PCI bus subsystem, leading to a hung task and an unusable system.

Could this sequence occur?

Thread 1 (driver unbind): Triggers driver removal. device_release_driver()
acquires device_lock(&pdev->dev) and calls vfio_unregister_group_dev(),
which blocks waiting for the device's open file descriptors to be closed
(waiting for device->comp completion).

Thread 2 (PCI bus reset): A concurrent bus reset calls pci_bus_lock(),
which acquires pci_dev_lock(bridge) and then iterates children to acquire
pci_dev_lock(pdev) (which uses device_lock(&pdev->dev)). It blocks waiting
for Thread 1.

Thread 3 (userspace close): Userspace closes the VFIO device FD. In
vfio_device_fops_release(), the driver calls vfio_pci_core_disable(), which
now blocks on pci_dev_lock(bridge) waiting for Thread 2.

Because Thread 3 is blocked, it never reaches vfio_device_put_registration()
to signal device->comp, causing Thread 1 to wait forever.

This race condition seems triggerable when an unprivileged user closes an
open VFIO FD concurrently with a root-initiated driver unbind and/or PCI
bus reset.

>  		if (pci_dev_trylock(pdev)) {
>  			if (!__pci_reset_function_locked(pdev))
>  				vdev->needs_reset = false;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908080946.2235849-1-YuanShang.Mao@amd.com?part=1