[bug report] PCI: hv: Fix a race condition when removing the device

[bug report] PCI: hv: Fix a race condition when removing the device

[Dan Carpenter]

Hello Long Li,

The patch 94d22763207a: "PCI: hv: Fix a race condition when removing
the device" from May 12, 2021, leads to the following
Smatch static checker warning:

	drivers/pci/controller/pci-hyperv.c:3294 hv_pci_bus_exit()
	warn: sleeping in atomic context

drivers/pci/controller/pci-hyperv.c
    3287 
    3288 	if (!keep_devs) {
    3289 		/* Delete any children which might still exist. */
    3290 		spin_lock_irqsave(&hbus->device_list_lock, flags);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This takes a spinlock.

    3291 		list_for_each_entry_safe(hpdev, tmp, &hbus->children, list_entry) {
    3292 			list_del(&hpdev->list_entry);
    3293 			if (hpdev->pci_slot)
--> 3294 				pci_destroy_slot(hpdev->pci_slot);
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The pci_destroy_slot() function takes a mutex and you can't take a mutex
when you're holding a spinlock because it can sleep.

    3295 			/* For the two refs got in new_pcichild_device() */
    3296 			put_pcichild(hpdev);
    3297 			put_pcichild(hpdev);
    3298 		}
    3299 		spin_unlock_irqrestore(&hbus->device_list_lock, flags);
    3300 	}
    3301 
    3302 	ret = hv_send_resources_released(hdev);
    3303 	if (ret) {
    3304 		dev_err(&hdev->device,

regards,
dan carpenter

RE: [bug report] PCI: hv: Fix a race condition when removing the device

[Long Li]

> Subject: [bug report] PCI: hv: Fix a race condition when removing the device
> 
> Hello Long Li,
> 
> The patch 94d22763207a: "PCI: hv: Fix a race condition when removing the
> device" from May 12, 2021, leads to the following Smatch static checker
> warning:
> 
> 	drivers/pci/controller/pci-hyperv.c:3294 hv_pci_bus_exit()
> 	warn: sleeping in atomic context
> 
> drivers/pci/controller/pci-hyperv.c
>     3287
>     3288 	if (!keep_devs) {
>     3289 		/* Delete any children which might still exist. */
>     3290 		spin_lock_irqsave(&hbus->device_list_lock, flags);
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This takes a spinlock.
> 
>     3291 		list_for_each_entry_safe(hpdev, tmp, &hbus->children,
> list_entry) {
>     3292 			list_del(&hpdev->list_entry);
>     3293 			if (hpdev->pci_slot)
> --> 3294 				pci_destroy_slot(hpdev->pci_slot);
>                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The
> pci_destroy_slot() function takes a mutex and you can't take a mutex when
> you're holding a spinlock because it can sleep.
> 
>     3295 			/* For the two refs got in new_pcichild_device() */
>     3296 			put_pcichild(hpdev);
>     3297 			put_pcichild(hpdev);
>     3298 		}
>     3299 		spin_unlock_irqrestore(&hbus->device_list_lock, flags);
>     3300 	}
>     3301
>     3302 	ret = hv_send_resources_released(hdev);
>     3303 	if (ret) {
>     3304 		dev_err(&hdev->device,
> 
> regards,
> dan carpenter

Thank you, Dan. I have submitted a patch to fix this.

Long