Subject: [PATCH -v5] PCI: Fix racing for pci device removing via sysfs From: Yinghai Lu Gu found nested removing through echo -n 1 > /sys/bus/pci/devices/0000\:10\:00.0/remove ; echo -n 1 > /sys/bus/pci/devices/0000\:1a\:01.0/remove will cause kernel crash as bus get freed. [ 418.946462] CPU 4 [ 418.968377] Pid: 512, comm: kworker/u:2 Tainted: G W 3.8.0 #2 FUJITSU-SV PRIMEQUEST 1800E/SB [ 419.081763] RIP: 0010:[] [] pci_bus_read_config_word+0x5e/0x90 [ 420.494137] Call Trace: [ 420.523326] [] ? remove_callback+0x1f/0x40 [ 420.591984] [] pci_pme_active+0x4b/0x1c0 [ 420.658545] [] pci_stop_bus_device+0x57/0xb0 [ 420.729259] [] pci_stop_and_remove_bus_device+0x16/0x30 [ 420.811392] [] remove_callback+0x2b/0x40 [ 420.877955] [] sysfs_schedule_callback_work+0x26/0x70 https://bugzilla.kernel.org/show_bug.cgi?id=54411 We have one patch that will let device hold bus ref to prevent it from being freed, but that will still generate warning. ------------[ cut here ]------------ WARNING: at lib/list_debug.c:53 __list_del_entry+0x63/0xd0() Hardware name: PRIMEQUEST 1800E list_del corruption, ffff8807d1b6c000->next is LIST_POISON1 (dead000000100100) Call Trace: [] warn_slowpath_common+0x7f/0xc0 [] warn_slowpath_fmt+0x46/0x50 [] __list_del_entry+0x63/0xd0 [] list_del+0x11/0x40 [] pci_destroy_dev+0x31/0xc0 [] pci_remove_bus_device+0x5b/0x70 [] pci_stop_and_remove_bus_device+0x1e/0x30 [] remove_callback+0x29/0x40 [] sysfs_schedule_callback_work+0x24/0x70 Bjorn pointed out that pci_dev should retain its reference to the pci_bus for as long as the pci_dev exists, so the release reference should go in pci_release_dev() instead. Here we move other bus_list change to pci_release_dev too to make it consistent with bus reference change. Also make stop device actually detach driver only, and remove device will do device_del instead. Then also could make hostbridge to use device_unregister to be pair with device_register before. At last we will not need to touch pci-sysfs bits. Reported-by: Gu Zheng Signed-off-by: Yinghai Lu --- drivers/pci/probe.c | 22 ++++++++++++++++++++++ drivers/pci/remove.c | 32 ++++++++------------------------ 2 files changed, 30 insertions(+), 24 deletions(-) Index: linux-2.6/drivers/pci/probe.c =================================================================== --- linux-2.6.orig/drivers/pci/probe.c +++ linux-2.6/drivers/pci/probe.c @@ -1119,6 +1119,20 @@ static void pci_release_capabilities(str pci_free_cap_save_buffers(dev); } +static void pci_free_resources(struct pci_dev *dev) +{ + int i; + + msi_remove_pci_irq_vectors(dev); + + pci_cleanup_rom(dev); + for (i = 0; i < PCI_NUM_RESOURCES; i++) { + struct resource *res = dev->resource + i; + if (res->parent) + release_resource(res); + } +} + /** * pci_release_dev - free a pci device structure when all users of it are finished. * @dev: device that's been disconnected @@ -1131,6 +1145,13 @@ static void pci_release_dev(struct devic struct pci_dev *pci_dev; pci_dev = to_pci_dev(dev); + + down_write(&pci_bus_sem); + list_del(&pci_dev->bus_list); + up_write(&pci_bus_sem); + pci_free_resources(pci_dev); + put_device(&pci_dev->bus->dev); + pci_release_capabilities(pci_dev); pci_release_of_node(pci_dev); kfree(pci_dev); @@ -1340,6 +1361,7 @@ void pci_device_add(struct pci_dev *dev, down_write(&pci_bus_sem); list_add_tail(&dev->bus_list, &bus->devices); up_write(&pci_bus_sem); + get_device(&bus->dev); ret = pcibios_add_device(dev); WARN_ON(ret < 0); Index: linux-2.6/drivers/pci/remove.c =================================================================== --- linux-2.6.orig/drivers/pci/remove.c +++ linux-2.6/drivers/pci/remove.c @@ -3,20 +3,6 @@ #include #include "pci.h" -static void pci_free_resources(struct pci_dev *dev) -{ - int i; - - msi_remove_pci_irq_vectors(dev); - - pci_cleanup_rom(dev); - for (i = 0; i < PCI_NUM_RESOURCES; i++) { - struct resource *res = dev->resource + i; - if (res->parent) - release_resource(res); - } -} - static void pci_stop_dev(struct pci_dev *dev) { pci_pme_active(dev, false); @@ -24,8 +10,7 @@ static void pci_stop_dev(struct pci_dev if (dev->is_added) { pci_proc_detach_device(dev); pci_remove_sysfs_dev_files(dev); - device_del(&dev->dev); - dev->is_added = 0; + device_release_driver(&dev->dev); } if (dev->bus->self) @@ -34,12 +19,11 @@ static void pci_stop_dev(struct pci_dev static void pci_destroy_dev(struct pci_dev *dev) { - down_write(&pci_bus_sem); - list_del(&dev->bus_list); - up_write(&pci_bus_sem); - - pci_free_resources(dev); - put_device(&dev->dev); + if (dev->is_added) { + device_del(&dev->dev); + put_device(&dev->dev); + dev->is_added = 0; + } } void pci_remove_bus(struct pci_bus *bus) @@ -126,7 +110,7 @@ void pci_stop_root_bus(struct pci_bus *b pci_stop_bus_device(child); /* stop the host bridge */ - device_del(&host_bridge->dev); + device_release_driver(&host_bridge->dev); } void pci_remove_root_bus(struct pci_bus *bus) @@ -145,5 +129,5 @@ void pci_remove_root_bus(struct pci_bus host_bridge->bus = NULL; /* remove the host bridge */ - put_device(&host_bridge->dev); + device_unregister(&host_bridge->dev); }