Commit 247fd50b authored by Peter Hurley's avatar Peter Hurley Committed by Stefan Richter

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 <mark.einon@gmail.com> 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:
 [<ffffffff81051c1f>] warn_slowpath_common+0x7f/0xc0
 [<ffffffff81051d16>] warn_slowpath_fmt+0x46/0x50
 [<ffffffff8103fa39>] ? default_spin_lock_flags+0x9/0x10
 [<ffffffff810df6b3>] __free_irq+0xa3/0x1e0
 [<ffffffff810df844>] free_irq+0x54/0xc0
 [<ffffffffa005a27e>] pci_remove+0x6e/0x210 [firewire_ohci]
 [<ffffffff8135ae7f>] pci_device_remove+0x3f/0x110
 [<ffffffff8141fdbc>] __device_release_driver+0x7c/0xe0
 [<ffffffff8141fe4c>] device_release_driver+0x2c/0x40
 [<ffffffff8141f5f1>] bus_remove_device+0xe1/0x120
 [<ffffffff8141cd1a>] device_del+0x12a/0x1c0
 [<ffffffff8141cdc6>] device_unregister+0x16/0x30
 [<ffffffff81354784>] pci_stop_bus_device+0x94/0xa0
 [<ffffffffa0091c67>] acpiphp_disable_slot+0xb7/0x1a0 [acpiphp]
 [<ffffffffa0090716>] ? get_slot_status+0x46/0xc0 [acpiphp]
 [<ffffffffa0091d7d>] acpiphp_check_bridge.isra.15+0x2d/0xf0 [acpiphp]
 [<ffffffffa0092442>] _handle_hotplug_event_bridge+0x372/0x4d0 [acpiphp]
 [<ffffffff81390f8c>] ? acpi_os_execute_deferred+0x2f/0x34
 [<ffffffff8116e22d>] ? kfree+0xed/0x110
 [<ffffffff8107086a>] process_one_work+0x12a/0x420
 [<ffffffffa00920d0>] ? _handle_hotplug_event_func+0x1d0/0x1d0 [acpiphp]
 [<ffffffff8107141e>] worker_thread+0x12e/0x2f0
 [<ffffffff810712f0>] ? manage_workers.isra.26+0x200/0x200
 [<ffffffff81075f13>] kthread+0x93/0xa0
 [<ffffffff8168d024>] kernel_thread_helper+0x4/0x10
 [<ffffffff81075e80>] ? kthread_freezable_should_stop+0x70/0x70
 [<ffffffff8168d020>] ? gs_change+0x13/0x13
Reported-by: default avatarMark Einon <mark.einon@gmail.com>
Signed-off-by: default avatarPeter Hurley <peter@hurleysoftware.com>
Signed-off-by: default avatarStefan Richter <stefanr@s5r6.in-berlin.de>
parent cfb0c9d1
...@@ -2246,7 +2246,6 @@ static int ohci_enable(struct fw_card *card, ...@@ -2246,7 +2246,6 @@ static int ohci_enable(struct fw_card *card,
const __be32 *config_rom, size_t length) const __be32 *config_rom, size_t length)
{ {
struct fw_ohci *ohci = fw_ohci(card); struct fw_ohci *ohci = fw_ohci(card);
struct pci_dev *dev = to_pci_dev(card->device);
u32 lps, version, irqs; u32 lps, version, irqs;
int i, ret; int i, ret;
...@@ -2382,24 +2381,6 @@ static int ohci_enable(struct fw_card *card, ...@@ -2382,24 +2381,6 @@ static int ohci_enable(struct fw_card *card,
reg_write(ohci, OHCI1394_AsReqFilterHiSet, 0x80000000); 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 | irqs = OHCI1394_reqTxComplete | OHCI1394_respTxComplete |
OHCI1394_RQPkt | OHCI1394_RSPkt | OHCI1394_RQPkt | OHCI1394_RSPkt |
OHCI1394_isochTx | OHCI1394_isochRx | OHCI1394_isochTx | OHCI1394_isochRx |
...@@ -3675,9 +3656,20 @@ static int pci_probe(struct pci_dev *dev, ...@@ -3675,9 +3656,20 @@ static int pci_probe(struct pci_dev *dev,
guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) | guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) |
reg_read(ohci, OHCI1394_GUIDLo); 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); err = fw_card_add(&ohci->card, max_receive, link_speed, guid);
if (err) if (err)
goto fail_contexts; goto fail_irq;
version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff;
dev_notice(&dev->dev, dev_notice(&dev->dev,
...@@ -3688,6 +3680,10 @@ static int pci_probe(struct pci_dev *dev, ...@@ -3688,6 +3680,10 @@ static int pci_probe(struct pci_dev *dev,
return 0; return 0;
fail_irq:
free_irq(dev->irq, ohci);
fail_msi:
pci_disable_msi(dev);
fail_contexts: fail_contexts:
kfree(ohci->ir_context_list); kfree(ohci->ir_context_list);
kfree(ohci->it_context_list); kfree(ohci->it_context_list);
...@@ -3763,8 +3759,6 @@ static int pci_suspend(struct pci_dev *dev, pm_message_t state) ...@@ -3763,8 +3759,6 @@ static int pci_suspend(struct pci_dev *dev, pm_message_t state)
int err; int err;
software_reset(ohci); software_reset(ohci);
free_irq(dev->irq, ohci);
pci_disable_msi(dev);
err = pci_save_state(dev); err = pci_save_state(dev);
if (err) { if (err) {
dev_err(&dev->dev, "pci_save_state failed\n"); dev_err(&dev->dev, "pci_save_state failed\n");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment