diff options
| author | Peter Hurley <peter@hurleysoftware.com> | 2013-03-27 06:59:58 -0400 |
|---|---|---|
| committer | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2013-04-28 17:36:44 -0400 |
| commit | 247fd50b5953d2b07832a07bd1d1c3b8e221fe9e (patch) | |
| tree | aad078070164b29a68e9f681ce34617d335eee31 /drivers/firewire | |
| parent | cfb0c9d1ffbf930a4a852f178b161c522b21b0ab (diff) | |
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: Mark Einon <mark.einon@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Diffstat (limited to 'drivers/firewire')
| -rw-r--r-- | drivers/firewire/ohci.c | 38 |
1 files changed, 16 insertions, 22 deletions
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index d9be53c1d806..48889353723f 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c | |||
| @@ -2246,7 +2246,6 @@ static int ohci_enable(struct fw_card *card, | |||
| 2246 | const __be32 *config_rom, size_t length) | 2246 | const __be32 *config_rom, size_t length) |
| 2247 | { | 2247 | { |
| 2248 | struct fw_ohci *ohci = fw_ohci(card); | 2248 | struct fw_ohci *ohci = fw_ohci(card); |
| 2249 | struct pci_dev *dev = to_pci_dev(card->device); | ||
| 2250 | u32 lps, version, irqs; | 2249 | u32 lps, version, irqs; |
| 2251 | int i, ret; | 2250 | int i, ret; |
| 2252 | 2251 | ||
| @@ -2382,24 +2381,6 @@ static int ohci_enable(struct fw_card *card, | |||
| 2382 | 2381 | ||
| 2383 | reg_write(ohci, OHCI1394_AsReqFilterHiSet, 0x80000000); | 2382 | reg_write(ohci, OHCI1394_AsReqFilterHiSet, 0x80000000); |
| 2384 | 2383 | ||
| 2385 | if (!(ohci->quirks & QUIRK_NO_MSI)) | ||
| 2386 | pci_enable_msi(dev); | ||
| 2387 | if (request_irq(dev->irq, irq_handler, | ||
| 2388 | pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, | ||
| 2389 | ohci_driver_name, ohci)) { | ||
| 2390 | dev_err(card->device, "failed to allocate interrupt %d\n", | ||
| 2391 | dev->irq); | ||
| 2392 | pci_disable_msi(dev); | ||
| 2393 | |||
| 2394 | if (config_rom) { | ||
| 2395 | dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE, | ||
| 2396 | ohci->next_config_rom, | ||
| 2397 | ohci->next_config_rom_bus); | ||
| 2398 | ohci->next_config_rom = NULL; | ||
| 2399 | } | ||
| 2400 | return -EIO; | ||
| 2401 | } | ||
| 2402 | |||
| 2403 | irqs = OHCI1394_reqTxComplete | OHCI1394_respTxComplete | | 2384 | irqs = OHCI1394_reqTxComplete | OHCI1394_respTxComplete | |
| 2404 | OHCI1394_RQPkt | OHCI1394_RSPkt | | 2385 | OHCI1394_RQPkt | OHCI1394_RSPkt | |
| 2405 | OHCI1394_isochTx | OHCI1394_isochRx | | 2386 | OHCI1394_isochTx | OHCI1394_isochRx | |
| @@ -3675,9 +3656,20 @@ static int pci_probe(struct pci_dev *dev, | |||
| 3675 | guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) | | 3656 | guid = ((u64) reg_read(ohci, OHCI1394_GUIDHi) << 32) | |
| 3676 | reg_read(ohci, OHCI1394_GUIDLo); | 3657 | reg_read(ohci, OHCI1394_GUIDLo); |
| 3677 | 3658 | ||
| 3659 | if (!(ohci->quirks & QUIRK_NO_MSI)) | ||
| 3660 | pci_enable_msi(dev); | ||
| 3661 | if (request_irq(dev->irq, irq_handler, | ||
| 3662 | pci_dev_msi_enabled(dev) ? 0 : IRQF_SHARED, | ||
| 3663 | ohci_driver_name, ohci)) { | ||
| 3664 | dev_err(&dev->dev, "failed to allocate interrupt %d\n", | ||
| 3665 | dev->irq); | ||
| 3666 | err = -EIO; | ||
| 3667 | goto fail_msi; | ||
| 3668 | } | ||
| 3669 | |||
| 3678 | err = fw_card_add(&ohci->card, max_receive, link_speed, guid); | 3670 | err = fw_card_add(&ohci->card, max_receive, link_speed, guid); |
| 3679 | if (err) | 3671 | if (err) |
| 3680 | goto fail_contexts; | 3672 | goto fail_irq; |
| 3681 | 3673 | ||
| 3682 | version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; | 3674 | version = reg_read(ohci, OHCI1394_Version) & 0x00ff00ff; |
| 3683 | dev_notice(&dev->dev, | 3675 | dev_notice(&dev->dev, |
| @@ -3688,6 +3680,10 @@ static int pci_probe(struct pci_dev *dev, | |||
| 3688 | 3680 | ||
| 3689 | return 0; | 3681 | return 0; |
| 3690 | 3682 | ||
| 3683 | fail_irq: | ||
| 3684 | free_irq(dev->irq, ohci); | ||
| 3685 | fail_msi: | ||
| 3686 | pci_disable_msi(dev); | ||
| 3691 | fail_contexts: | 3687 | fail_contexts: |
| 3692 | kfree(ohci->ir_context_list); | 3688 | kfree(ohci->ir_context_list); |
| 3693 | kfree(ohci->it_context_list); | 3689 | kfree(ohci->it_context_list); |
| @@ -3763,8 +3759,6 @@ static int pci_suspend(struct pci_dev *dev, pm_message_t state) | |||
| 3763 | int err; | 3759 | int err; |
| 3764 | 3760 | ||
| 3765 | software_reset(ohci); | 3761 | software_reset(ohci); |
| 3766 | free_irq(dev->irq, ohci); | ||
| 3767 | pci_disable_msi(dev); | ||
| 3768 | err = pci_save_state(dev); | 3762 | err = pci_save_state(dev); |
| 3769 | if (err) { | 3763 | if (err) { |
| 3770 | dev_err(&dev->dev, "pci_save_state failed\n"); | 3764 | dev_err(&dev->dev, "pci_save_state failed\n"); |
