Re: [PATCH v11 2/2] PCI: Fix AB-BA deadlock between device_lock and pci_rescan_remove_lock in remove_store

[Niklas Schnelle]

On Thu, 2026-03-26 at 10:35 +0200, Ionut Nechita (Wind River) wrote:
> remove_store() calls pci_stop_and_remove_bus_device_locked() which
> takes pci_rescan_remove_lock first, then device_lock during driver
> release.  Meanwhile, unbind_store() takes device_lock first (via
> device_driver_detach), and the driver's .remove() callback may call
> pci_disable_sriov() -> sriov_del_vfs() -> pci_lock_rescan_remove().
> 
> This creates an AB-BA deadlock:
> 
>   CPU0 (remove_store)               CPU1 (unbind_store)
>   --------------------              --------------------
>   pci_lock_rescan_remove()
>                                     device_lock()
>                                     driver .remove()
>                                       sriov_del_vfs()
>                                         pci_lock_rescan_remove()  <-- WAITS
>   pci_stop_bus_device()
>     device_release_driver()
>       device_lock()                                               <-- WAITS
> 
> Fix this by first marking the device as dead using kill_device() to
> prevent any new driver from binding, then calling device_release_driver()
> before pci_stop_and_remove_bus_device_locked().
> 
> Marking the device dead closes the race window between unbinding and
> removal where a new driver could theoretically bind: once the dead flag
> is set, the device core will refuse any new driver probe.
> 
> After device_release_driver() returns, the driver is already unbound,
> so the subsequent device_release_driver() call inside
> pci_stop_and_remove_bus_device_locked() becomes a no-op.
> 
> Fixes: a5338e365c45 ("PCI/IOV: Fix race between SR-IOV enable/disable and hotplug")
> Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-pci/0ca9e675-478c-411d-be32-e2d81439288f@xxxxxxxxxxxx/
> Reported-by: Benjamin Block <bblock@xxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-pci/20260317090149.GA3835708@xxxxxxxxxxxxxxxxxx/
> Suggested-by: Benjamin Block <bblock@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Ionut Nechita <ionut.nechita@xxxxxxxxxxxxx>
> ---
>  drivers/pci/pci-sysfs.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index a2f8a5d6190fd..e87aa96c02bde 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -518,8 +518,36 @@ 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) and then
> +		 * device_lock during driver release, while a concurrent
> +		 * unbind_store (or sriov_numvfs_store) takes device_lock
> +		 * first and then pci_rescan_remove_lock (via
> +		 * sriov_del_vfs), creating a circular dependency.
> +		 *
> +		 * By unbinding first, the driver's .remove() callback
> +		 * (including any SR-IOV VF cleanup) completes before
> +		 * pci_rescan_remove_lock is acquired, ensuring both
> +		 * paths take locks in the same order.
> +		 */
> +		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 looks good to me and this use of kill_device() seems to match
pretty well with the comment in kill_device() as setting it on tearing
things down. Thank you for working on this rats nest of PCI
rescan/remove lock issues!

Feel free to add my

Reviewed-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>