PCI: pciehp: Fix use-after-free on unplug

BugLink: https://bugs.launchpad.net/bugs/1835845
commit 281e878eab191cce4259abbbf1a0322e3adae02c upstream.

When pciehp is unbound (e.g. on unplug of a Thunderbolt device), the
hotplug_slot struct is deregistered and thus freed before freeing the
IRQ.  The IRQ handler and the work items it schedules print the slot
name referenced from the freed structure in various informational and
debug log messages, each time resulting in a quadruple dereference of
freed pointers (hotplug_slot -> pci_slot -> kobject -> name).

At best the slot name is logged as "(null)", at worst kernel memory is
exposed in logs or the driver crashes:

  pciehp 0000:10:00.0:pcie204: Slot((null)): Card not present

An attacker may provoke the bug by unplugging multiple devices on a
Thunderbolt daisy chain at once.  Unplugging can also be simulated by
powering down slots via sysfs.  The bug is particularly easy to trigger
in poll mode.

It has been present since the driver's introduction in 2004:
https://git.kernel.org/tglx/history/c/c16b4b14d980

Fix by rearranging teardown such that the IRQ is freed first.  Run the
work items queued by the IRQ handler to completion before freeing the
hotplug_slot struct by draining the work queue from the ->release_slot
callback which is invoked by pci_hp_deregister().

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: stable@vger.kernel.org # v2.6.4
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index e7d6cfaf386581a7d89541c89298d235bde752d6..9fc4357c3db940312c7caf86e3e5cab8866316a7 100644 (file)
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -132,6 +132,7 @@ int pciehp_unconfigure_device(struct slot *p_slot);
 void pciehp_queue_pushbutton_work(struct work_struct *work);
 struct controller *pcie_init(struct pcie_device *dev);
 int pcie_init_notification(struct controller *ctrl);
+void pcie_shutdown_notification(struct controller *ctrl);
 int pciehp_enable_slot(struct slot *p_slot);
 int pciehp_disable_slot(struct slot *p_slot);
 void pcie_reenable_notification(struct controller *ctrl);

diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 1288289cc85d38fa09190586f7743ac1271e5a83..c38e392d63e4b2037b1c1cbb879b3d1239ec8a80 100644 (file)
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -76,6 +76,12 @@ static int reset_slot(struct hotplug_slot *slot, int probe);
  */
 static void release_slot(struct hotplug_slot *hotplug_slot)
 {
+       struct slot *slot = hotplug_slot->private;
+
+       /* queued work needs hotplug_slot name */
+       cancel_delayed_work(&slot->work);
+       drain_workqueue(slot->wq);
+
        kfree(hotplug_slot->ops);
        kfree(hotplug_slot->info);
        kfree(hotplug_slot);
@@ -278,6 +284,7 @@ static void pciehp_remove(struct pcie_device *dev)
 {
        struct controller *ctrl = get_service_data(dev);
 
+       pcie_shutdown_notification(ctrl);
        cleanup_slot(ctrl);
        pciehp_release_ctrl(ctrl);
 }

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 8e653ffbda07b375d658c5e33b7bc6bda323f433..2a9a80ddaa62547ee175d671af31fca2b57e1f51 100644 (file)
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -785,7 +785,7 @@ int pcie_init_notification(struct controller *ctrl)
        return 0;
 }
 
-static void pcie_shutdown_notification(struct controller *ctrl)
+void pcie_shutdown_notification(struct controller *ctrl)
 {
        if (ctrl->notification_enabled) {
                pcie_disable_notification(ctrl);
@@ -820,7 +820,7 @@ abort:
 static void pcie_cleanup_slot(struct controller *ctrl)
 {
        struct slot *slot = ctrl->slot;
-       cancel_delayed_work(&slot->work);
+
        destroy_workqueue(slot->wq);
        kfree(slot);
 }
@@ -913,7 +913,6 @@ abort:
 
 void pciehp_release_ctrl(struct controller *ctrl)
 {
-       pcie_shutdown_notification(ctrl);
        pcie_cleanup_slot(ctrl);
        kfree(ctrl);
 }