Re: [PATCH] PCI: Fix AB-BA deadlock between remove and SR-IOV sysfs

[Niklas Schnelle]

On Sun, 2026-07-05 at 23:24 +0800, Guangshuo Li wrote:
> sriov_numvfs_store() holds the PF device lock while calling the
> driver's sriov_configure() callback. The SR-IOV configure path may add
> or remove VFs under pci_rescan_remove_lock.
> 
> The PCI remove sysfs path takes the locks in the opposite order.
> remove_store() calls pci_stop_and_remove_bus_device_locked(), which takes
> pci_rescan_remove_lock and then releases the device driver, requiring
> device_lock() on the device being removed.
> 
> If a write to sriov_numvfs races with removal of the same PF, or removal
> of an upstream bridge that reaches the PF, the two paths can deadlock:
> 
>   sriov_numvfs_store()       remove_store()
>   --------------------       --------------
>   device_lock()
>                              pci_rescan_remove_lock
>   pci_rescan_remove_lock
>                              device_lock()
> 
> Avoid the reversed locking order by unbinding the driver before entering
> pci_stop_and_remove_bus_device_locked(). Mark the device dead first so a
> new driver cannot bind between the unbind and the removal. After the
> driver has been unbound, the SR-IOV cleanup in the driver's remove path
> has completed before pci_rescan_remove_lock is acquired.
> 
> Fixes: a5338e365c45 ("PCI/IOV: Fix race between SR-IOV enable/disable and hotplug")
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>  drivers/pci/pci-sysfs.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 5ec0b245a69b..5eae72508129 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -519,8 +519,35 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *attr,
>  	if (kstrtoul(buf, 0, &val) < 0)
>  		return -EINVAL;
>  
> -	if (val && device_remove_file_self(dev, attr))
> +	if (val && device_remove_file_self(dev, attr)) {
> +		/*
> +		 * Mark the device as dead so that no new driver can bind
> +		 * between the unbind and the removal below. Once the dead
> +		 * flag is set, the device core will refuse any new driver
> +		 * probe.
> +		 */
> +		device_lock(dev);
> +		kill_device(dev);
> +		device_unlock(dev);
> +
> +		/*
> +		 * Unbind the driver before removing the device to avoid an
> +		 * AB-BA deadlock between device_lock() and
> +		 * pci_rescan_remove_lock. Without this, remove_store() takes
> +		 * pci_rescan_remove_lock first via
> +		 * pci_stop_and_remove_bus_device_locked(), then takes
> +		 * device_lock() during driver release, while a concurrent
> +		 * sriov_numvfs_store() takes device_lock() first and then
> +		 * pci_rescan_remove_lock via SR-IOV VF removal.
> +		 *
> +		 * By unbinding first, the driver's .remove() callback,
> +		 * including any SR-IOV VF cleanup, completes before
> +		 * pci_rescan_remove_lock is acquired.
> +		 */
> +		device_release_driver(dev);
>  		pci_stop_and_remove_bus_device_locked(to_pci_dev(dev));
> +	}
> +
>  	return count;
>  }
>  static DEVICE_ATTR_IGNORE_LOCKDEP(remove, 0220, NULL,

This seems to be a complete duplicate of Ionut Nechita's patch[0] from
April. The comments are close enough, but not identical, that I don't
think this can really have been created independently. Interestingly
though your patch refers to the race with sriov_numvfs_store() while
Ionut's patch refers to an equivalent race with unbind_store().  So
this makes it odd for straight up plagiarism and I wonder if Ionut's
patches have been public for long enough that they ended up in AI
knowledge. Then some AI might have reproduced it so closely in which
case you should at least disclose the AI use. Either way the patch by
Ionut should be taken for being the original source of this fix. And
technically the fix remains valid from my point of view and my R-b on
Ionut's version stands.

Thanks,
Niklas

[0]
https://lore.kernel.org/all/b378529b4afc4f2a6e393498fe9b9b7f056f95c4.1776755661.git.ionut.nechita@xxxxxxxxxxxxx/