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

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

[Guangshuo Li]

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@gmail.com>
---
 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,
-- 
2.43.0

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@gmail.com>
> ---
>  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@windriver.com/

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

[Benjamin Block]

On Mon, Jul 06, 2026 at 10:16:36AM +0200, Niklas Schnelle wrote:
> On Sun, 2026-07-05 at 23:24 +0800, Guangshuo Li wrote:
> > 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.

Yep, +1.
This is not helpful. We still want the original patchset included (I sent a
ping a few minutes ago), but then in its complete form with the proper
authors.

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Ehningen     /     Registergericht: AmtsG Stuttgart, HRB 243294

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

[Krzysztof Wilczyński]

Hello,

> > 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.
> 
> Yep, +1.
> This is not helpful. We still want the original patchset included (I sent a
> ping a few minutes ago), but then in its complete form with the proper
> authors.

Older patches (include reviews and conversations about the problem) for reference:

  - https://lore.kernel.org/linux-pci/20240610220304.3162895-2-kbusch@meta.com/
  - https://lore.kernel.org/linux-pci/20240612181625.3604512-1-kbusch@meta.com/
  - https://lore.kernel.org/linux-pci/20251030-revert_sriov_lock-v1-0-70f82ade426f@linux.ibm.com/
  - https://lore.kernel.org/linux-pci/20251119-revert_sriov_lock-v2-0-ea50eb1e8f96@linux.ibm.com/
  - https://lore.kernel.org/linux-pci/20251216-revert_sriov_lock-v3-0-dac4925a7621@linux.ibm.com/
  - https://lore.kernel.org/linux-pci/20260214193235.262219-3-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260219212648.82606-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260225202434.18737-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260228120138.51197-2-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260303080903.28693-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260306082108.17322-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260308135352.80346-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260309194920.16459-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260310074303.17480-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260318210316.61975-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/20260326083534.23602-1-ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/cover.1776755661.git.ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/cover.1776756380.git.ionut.nechita@windriver.com/
  - https://lore.kernel.org/linux-pci/cover.1776839248.git.ionut.nechita@windriver.com/

This patch is a no-go, of course, given all the prior work (as far as 2024)
that had been done to fix this problem.

Thank you,

	Krzysztof