From: "Schmid, Carsten" <Carsten_Schmid@mentor.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: [Resend] [PATCH v3] usb: xhci-pci: reorder removal to avoid use-after-free
Date: Fri, 16 Aug 2019 09:03:20 +0000 [thread overview]
Message-ID: <1565946200239.8897@mentor.com> (raw)
On driver removal, the platform_device_unregister call
attached through devm_add_action_or_reset was executed
after usb_hcd_pci_remove.
This lead to a use-after-free for the iomem resource of
the xhci-ext-caps driver in the platform removal
because the parent of the resource was freed earlier.
Fix this by reordering of the removal sequence.
Signed-off-by: Carsten Schmid <carsten_schmid@mentor.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Carsten Schmid <carsten_schmid@mentor.com>
---
v2:
- more speaking name for private data element
- consider failure in driver init sequence
- fix minor issues found by checkpatch.pl
v3:
- corrected typo: resorce -> resource
- added Reviewed by and Tested-by
Resend v3:
- made a mistake when copying patch content to mail in v3
---
drivers/usb/host/xhci-ext-caps.c | 25 +++++++++++++++----------
drivers/usb/host/xhci-pci.c | 8 +++++++-
drivers/usb/host/xhci-pci.h | 20 ++++++++++++++++++++
drivers/usb/host/xhci.h | 1 +
4 files changed, 43 insertions(+), 11 deletions(-)
create mode 100644 drivers/usb/host/xhci-pci.h
diff --git a/drivers/usb/host/xhci-ext-caps.c b/drivers/usb/host/xhci-ext-caps.c
index 399113f9fc5c..28a7d53ecf2c 100644
--- a/drivers/usb/host/xhci-ext-caps.c
+++ b/drivers/usb/host/xhci-ext-caps.c
@@ -7,21 +7,19 @@
#include <linux/platform_device.h>
#include "xhci.h"
+#include "xhci-pci.h"
#define USB_SW_DRV_NAME "intel_xhci_usb_sw"
#define USB_SW_RESOURCE_SIZE 0x400
-static void xhci_intel_unregister_pdev(void *arg)
-{
- platform_device_unregister(arg);
-}
-
static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd *xhci, u32 cap_offset)
{
struct usb_hcd *hcd = xhci_to_hcd(xhci);
struct device *dev = hcd->self.controller;
struct platform_device *pdev;
struct resource res = { 0, };
+ struct xhci_pci_priv *priv = (struct xhci_pci_priv *)xhci->priv;
+
int ret;
pdev = platform_device_alloc(USB_SW_DRV_NAME, PLATFORM_DEVID_NONE);
@@ -52,11 +50,7 @@ static int xhci_create_intel_xhci_sw_pdev(struct xhci_hcd *xhci, u32 cap_offset)
return ret;
}
- ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
- if (ret) {
- dev_err(dev, "couldn't add unregister action for intel_xhci_usb_sw pdev\n");
- return ret;
- }
+ priv->role_switch_pdev = pdev;
return 0;
}
@@ -88,3 +82,14 @@ int xhci_ext_cap_init(struct xhci_hcd *xhci)
return 0;
}
EXPORT_SYMBOL_GPL(xhci_ext_cap_init);
+
+void xhci_ext_cap_remove(struct xhci_hcd *xhci)
+{
+ struct xhci_pci_priv *priv = (struct xhci_pci_priv *)xhci->priv;
+
+ if (priv->role_switch_pdev) {
+ platform_device_unregister(priv->role_switch_pdev);
+ priv->role_switch_pdev = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(xhci_ext_cap_remove);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c2fe218e051f..f2201f380c17 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include "xhci.h"
+#include "xhci-pci.h"
#include "xhci-trace.h"
#define SSIC_PORT_NUM 2
@@ -62,6 +63,7 @@ static struct hc_driver __read_mostly xhci_pci_hc_driver;
static int xhci_pci_setup(struct usb_hcd *hcd);
static const struct xhci_driver_overrides xhci_pci_overrides __initconst = {
+ .extra_priv_size = sizeof(struct xhci_pci_priv),
.reset = xhci_pci_setup,
};
@@ -350,7 +352,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
retval = usb_add_hcd(xhci->shared_hcd, dev->irq,
IRQF_SHARED);
if (retval)
- goto put_usb3_hcd;
+ goto remove_ext_cap;
/* Roothub already marked as USB 3.0 speed */
if (!(xhci->quirks & XHCI_BROKEN_STREAMS) &&
@@ -368,6 +370,8 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
return 0;
+remove_ext_cap:
+ xhci_ext_cap_remove(xhci);
put_usb3_hcd:
usb_put_hcd(xhci->shared_hcd);
dealloc_usb2_hcd:
@@ -393,6 +397,8 @@ static void xhci_pci_remove(struct pci_dev *dev)
xhci->shared_hcd = NULL;
}
+ xhci_ext_cap_remove(xhci);
+
/* Workaround for spurious wakeups at shutdown with HSW */
if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
pci_set_power_state(dev, PCI_D3hot);
diff --git a/drivers/usb/host/xhci-pci.h b/drivers/usb/host/xhci-pci.h
new file mode 100644
index 000000000000..fc0cde231679
--- /dev/null
+++ b/drivers/usb/host/xhci-pci.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * xhci-pci.h - xHCI extended capability handling platform Glue.
+ *
+ * Copyright (C) 2019 Mentor Graphics (Deutschland) GmbH
+ * Derived from xhci-plat.h
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#ifndef _XHCI_PCI_H
+#define _XHCI_PCI_H
+
+struct xhci_pci_priv {
+ struct platform_device *role_switch_pdev;
+};
+
+#endif /* _XHCI_PCI_H */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index fabbce1c542a..847d2021fc2c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2052,6 +2052,7 @@ void xhci_init_driver(struct hc_driver *drv,
const struct xhci_driver_overrides *over);
int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id);
int xhci_ext_cap_init(struct xhci_hcd *xhci);
+void xhci_ext_cap_remove(struct xhci_hcd *xhci);
int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);
int xhci_resume(struct xhci_hcd *xhci, bool hibernated);
--
2.17.1
next reply other threads:[~2019-08-16 9:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-16 9:03 Schmid, Carsten [this message]
2019-08-22 15:23 ` [Resend] [PATCH v3] usb: xhci-pci: reorder removal to avoid use-after-free Mathias Nyman
2019-08-22 15:25 ` Hans de Goede
2019-08-22 15:48 ` AW: " Schmid, Carsten
2019-08-22 20:14 ` Hans de Goede
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1565946200239.8897@mentor.com \
--to=carsten_schmid@mentor.com \
--cc=hdegoede@redhat.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.