Date: Wed, 27 Mar 2013 06:59:58 -0400 From: Peter Hurley Subject: firewire: ohci: Fix double free_irq() A pci device can be removed while in its suspended state. Because the ohci driver freed the irq to suspend, free_irq() is called twice; once from pci_remove() and again from pci_suspend(), which issues the warning below [1]. Rather than allocate the irq in the .enable() path, move the allocation to .probe(). Consequently, the irq is not reallocated upon pci_resume() and thus is not freed upon pci_suspend(). [1] Warning reported by Mark Einon when suspending an MSI MS-1727 GT740 laptop on Ubuntu 3.5.0-22-generic WARNING: at ./kernel/irq/manage.c:1198 __free_irq+0xa3/0x1e0() Hardware name: MS-1727 Trying to free already-free IRQ 16 Modules linked in: ip6table_filter ip6_tables ebtable_nat ebtables <...snip...> Pid: 4, comm: kworker/0:0 Tainted: P O 3.5.0-22-generic #34-Ubuntu Call Trace: [] warn_slowpath_common+0x7f/0xc0 [] warn_slowpath_fmt+0x46/0x50 [] ? default_spin_lock_flags+0x9/0x10 [] __free_irq+0xa3/0x1e0 [] free_irq+0x54/0xc0 [] pci_remove+0x6e/0x210 [firewire_ohci] [] pci_device_remove+0x3f/0x110 [] __device_release_driver+0x7c/0xe0 [] device_release_driver+0x2c/0x40 [] bus_remove_device+0xe1/0x120 [] device_del+0x12a/0x1c0 [] device_unregister+0x16/0x30 [] pci_stop_bus_device+0x94/0xa0 [] acpiphp_disable_slot+0xb7/0x1a0 [acpiphp] [] ? get_slot_status+0x46/0xc0 [acpiphp] [] acpiphp_check_bridge.isra.15+0x2d/0xf0 [acpiphp] [] _handle_hotplug_event_bridge+0x372/0x4d0 [acpiphp] [] ? acpi_os_execute_deferred+0x2f/0x34 [] ? kfree+0xed/0x110 [] process_one_work+0x12a/0x420 [] ? _handle_hotplug_event_func+0x1d0/0x1d0 [acpiphp] [] worker_thread+0x12e/0x2f0 [] ? manage_workers.isra.26+0x200/0x200 [] kthread+0x93/0xa0 [] kernel_thread_helper+0x4/0x10 [] ? kthread_freezable_should_stop+0x70/0x70 [] ? gs_change+0x13/0x13 Reported-by: Mark Einon Signed-off-by: Peter Hurley Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2246,7 +2246,6 @@ static int ohci_enable(struct fw_card *c const __be32 *config_rom, size_t length) { struct fw_ohci *ohci = fw_ohci(card); - struct pci_dev *dev = to_pci_dev(card->device); u32 lps, version, irqs; int i, ret; @@ -2382,24 +2381,6 @@ static int ohci_enable(struct fw_card *c reg_write(ohci, OHCI1394_AsReqFilterHiSet, 0x80000000); - if (!(ohci->quirks & QUIRK_NO_MSI)) - pci_enable_msi(dev); - if (request_irq(dev->irq, irq_handler, - pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, - ohci_driver_name, ohci)) { - dev_err(card->device, "failed to allocate interrupt %d\n", - dev->irq); - pci_disable_msi(dev); - - if (config_rom) { - dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, - ohci->next_config_rom, - ohci->next_config_rom_bus); - ohci->next_config_rom = NULL; - } - return -EIO; - } - irqs = OHCI1394_reqTxComplete | OHCI1394_respTxComplete | OHCI1394_RQPkt | OHCI1394_RSPkt | OHCI1394_isochTx | OHCI1394_isochRx | @@ -3675,9 +3656,20 @@ static int pci_probe(struct pci_dev *dev guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) | reg_read(ohci, OHCI1394_GUIDLo); + if (!(ohci->quirks & QUIRK_NO_MSI)) + pci_enable_msi(dev); + if (request_irq(dev->irq, irq_handler, + pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, + ohci_driver_name, ohci)) { + dev_err(&dev->dev, "failed to allocate interrupt %d\n", + dev->irq); + err = -EIO; + goto fail_msi; + } + err = fw_card_add(&ohci->card, max_receive, link_speed, guid); if (err) - goto fail_contexts; + goto fail_irq; version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; dev_notice(&dev->dev, @@ -3688,6 +3680,10 @@ static int pci_probe(struct pci_dev *dev return 0; + fail_irq: + free_irq(dev->irq, ohci); + fail_msi: + pci_disable_msi(dev); fail_contexts: kfree(ohci->ir_context_list); kfree(ohci->it_context_list); @@ -3763,8 +3759,6 @@ static int pci_suspend(struct pci_dev *d int err; software_reset(ohci); - free_irq(dev->irq, ohci); - pci_disable_msi(dev); err = pci_save_state(dev); if (err) { dev_err(&dev->dev, "pci_save_state failed\n");