Skip to contentSkip to conversation

PCI: hv: Fix a race condition when removing the device

bug report Dan Carpenter 2 participants 2 messages Aug 23, 2021 – Aug 24, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 39 lines

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,
Quoted 2 lines

regards,
dan carpenter

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

Long