usb: xhci-pci: reorder removal to avoid use-after-free

PATCH Superseded Schmid, Carsten 2 participants 6 messages Aug 14, 2019 – Aug 15, 2019

Subsystems (3)

USB PCI USB xHCI driver

lore

Share & tools

lore

Schmid, Carsten

author

2019-08-14 11:39 UTC #0 lore raw

PATCH 1/1 usb: xhci-pci: reorder removal to avoid use-after-free

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 resorce 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 <[email protected]>

---
 drivers/usb/host/xhci-ext-caps.c | 22 ++++++++++++----------
 drivers/usb/host/xhci-pci.c      |  4 ++++
 drivers/usb/host/xhci-pci.h      | 19 +++++++++++++++++++
 drivers/usb/host/xhci.h          |  1 +
 4 files changed, 36 insertions(+), 10 deletions(-)
 create mode 100644 drivers/usb/host/xhci-pci.h

drivers/usb/host/xhci-ext-caps.c +12 ~~-10~~

index 399113f9fc5c..d2ab1e2a39c0 100644
@@ -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->pdev = pdev;
 
 	return 0;
 }

@@ -88,3 +82,11 @@ 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->pdev)
+		platform_device_unregister(priv->pdev);
+}
+EXPORT_SYMBOL_GPL(xhci_ext_cap_remove);

drivers/usb/host/xhci-pci.c +4 -0

index c2fe218e051f..a4d094df56f7 100644
@@ -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,
 };

@@ -393,6 +395,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);

drivers/usb/host/xhci-pci.h new file +19 -0

new file mode 100644
index 000000000000..ead9618d7368
@@ -0,0 +1,19 @@
+/*
+ * 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 *pdev;
+};
+
+#endif	/* _XHCI_PCI_H */

drivers/usb/host/xhci.h +1 -0

index fabbce1c542a..847d2021fc2c 100644
@@ -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);

Signature · 1 line

2.17.1

Hans de Goede

2019-08-14 12:54 UTC #1 lore raw

Hi,

Quoted 10 lines · Carsten

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 resorce 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 <[email protected]>

Assuming this has been tested, overal this looks good to me.

But there are 2 things to fix:

1) Maybe pick a more descriptive struct member name then pdev. pdev with pci-devices often points to a pci_device ... How about: role_switch_pdev ?

2) xhci_ext_cap_init() is not the last call which can fail in xhci_pci_probe(), since you now no longer use devm_add_action_or_reset for auto-cleanup, you must now manually cleanup by calling xhci_ext_cap_remove() when later steps of xhci_pci_probe() fail. it looks like you will need a new ext_cap_remove error-exit label for this put above the put_usb3_hcd label and goto this new label instead of to put_usb3_hcd in all error paths after a successful call to xhci_ext_cap_init()

Regards,

Hans

Quoted 129 lines

---
  drivers/usb/host/xhci-ext-caps.c | 22 ++++++++++++----------
  drivers/usb/host/xhci-pci.c      |  4 ++++
  drivers/usb/host/xhci-pci.h      | 19 +++++++++++++++++++
  drivers/usb/host/xhci.h          |  1 +
  4 files changed, 36 insertions(+), 10 deletions(-)

create mode 100644 drivers/usb/host/xhci-pci.h

drivers/usb/host/xhci-ext-caps.c +12 ~~-10~~

index 399113f9fc5c..d2ab1e2a39c0 100644
@@ -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->pdev = pdev;
 
  	return 0;
  }

@@ -88,3 +82,11 @@ 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->pdev)
+		platform_device_unregister(priv->pdev);
+}
+EXPORT_SYMBOL_GPL(xhci_ext_cap_remove);

drivers/usb/host/xhci-pci.c +4 -0

index c2fe218e051f..a4d094df56f7 100644
@@ -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,
  };

@@ -393,6 +395,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);

drivers/usb/host/xhci-pci.h new file +19 -0

new file mode 100644
index 000000000000..ead9618d7368
@@ -0,0 +1,19 @@
+/*
+ * 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 *pdev;
+};
+
+#endif	/* _XHCI_PCI_H */

drivers/usb/host/xhci.h +1 -0

index fabbce1c542a..847d2021fc2c 100644
@@ -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);

Schmid, Carsten

author

2019-08-14 13:32 UTC #2 lore raw

Quoted 7 lines

Quoted 7 lines
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 resorce of the xhci-ext-caps driver in the platform removal because the parent of the resource was freed earlier.

Quoted 5 lines

Quoted 3 lines
Fix this by reordering of the removal sequence.
Signed-off-by Carsten Schmid <[email protected]>
Assuming this has been tested, overal this looks good to me.

Tested on 4.14.129, ported to v5.2.7, compiled there.

Quoted 6 lines

But there are 2 things to fix:
1) Maybe pick a more descriptive struct member name then pdev. pdev with pci-devices often points to a pci_device ... How about: role_switch_pdev ?

Ok, good point. Had platform dev pdev in mind ...

Quoted 10 lines

2) xhci_ext_cap_init() is not the last call which can fail in xhci_pci_probe(), since you now no longer use
devm_add_action_or_reset
for auto-cleanup, you must now manually cleanup by calling xhci_ext_cap_remove() when later steps of xhci_pci_probe() fail. it looks like you will need a new ext_cap_remove error-exit label for this put above the put_usb3_hcd label and goto this new label instead of to put_usb3_hcd in all error paths after a successful call to xhci_ext_cap_init()

Right. Will review this path and correct accordingly.

Maybe an additional label isn't required because pdev is only set when xhci_ext_cap_init created the platform device, and xhci_ext_cap_remove checks for pdev being set.
So a call to xhci_ext_cap_remove doesn't harm if pdev is not set up yet. But for readability it might be better to create a label.

Best regards
Carsten

Hans de Goede

2019-08-14 13:36 UTC #3 lore raw

Hi,

Quoted 35 lines · Carsten

Quoted 7 lines
Quoted 7 lines
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 resorce of the xhci-ext-caps driver in the platform removal because the parent of the resource was freed earlier.
Quoted 5 lines
Quoted 3 lines
Fix this by reordering of the removal sequence.
Signed-off-by Carsten Schmid <[email protected]>
Assuming this has been tested, overal this looks good to me.
Tested on 4.14.129, ported to v5.2.7, compiled there.
Quoted 6 lines
But there are 2 things to fix:
1) Maybe pick a more descriptive struct member name then pdev. pdev with pci-devices often points to a pci_device ... How about: role_switch_pdev ?
Ok, good point. Had platform dev pdev in mind ...
Quoted 10 lines
2) xhci_ext_cap_init() is not the last call which can fail in xhci_pci_probe(), since you now no longer use
devm_add_action_or_reset
for auto-cleanup, you must now manually cleanup by calling xhci_ext_cap_remove() when later steps of xhci_pci_probe() fail. it looks like you will need a new ext_cap_remove error-exit label for this put above the put_usb3_hcd label and goto this new label instead of to put_usb3_hcd in all error paths after a successful call to xhci_ext_cap_init()

Quoted 7 lines

Right. Will review this path and correct accordingly.
Maybe an additional label isn't required because pdev is only set when xhci_ext_cap_init created the platform device, and xhci_ext_cap_remove checks for pdev being set.
So a call to xhci_ext_cap_remove doesn't harm if pdev is not set up yet. But for readability it might be better to create a label.

Right, when taking a quick look myself I realized that an extra label would not be necessary, but not having the extra label will confuse the reader of the code, since now we are undoing something which we did not do, so I would prefer if you use the extra label.

Regards,

Hans

Schmid, Carsten

author

2019-08-14 14:32 UTC #4 lore raw

PATCH v2 1/1 usb: xhci-pci: reorder removal to avoid use-after-free

Fix this by reordering of the removal sequence.

Signed-off-by Carsten Schmid <[email protected]>

---
v2:
- more speaking name for private data element
- consider failure in driver init sequence
- fix minor issues found by checkpatch.pl

---
 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

drivers/usb/host/xhci-ext-caps.c +15 ~~-10~~

index 399113f9fc5c..28a7d53ecf2c 100644
@@ -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);

drivers/usb/host/xhci-pci.c +7 -1

index c2fe218e051f..f2201f380c17 100644
@@ -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);

drivers/usb/host/xhci-pci.h new file +20 -0

new file mode 100644
index 000000000000..fc0cde231679
@@ -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 */

drivers/usb/host/xhci.h +1 -0

index fabbce1c542a..847d2021fc2c 100644
@@ -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);

Signature · 1 line

2.17.1

Hans de Goede

Reviewed-by on v2 1/1

2019-08-15 15:27 UTC #5 lore raw

Hi,

Quoted 10 lines · Carsten

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 resorce 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 <[email protected]>

Patch looks good to me:

Reviewed-by Hans de Goede <[email protected]>

Regards,

Hans

Quoted 156 lines

---
v2:
- more speaking name for private data element
- consider failure in driver init sequence
- fix minor issues found by checkpatch.pl

---
  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

drivers/usb/host/xhci-ext-caps.c +15 ~~-10~~

index 399113f9fc5c..28a7d53ecf2c 100644
@@ -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);

drivers/usb/host/xhci-pci.c +7 -1

index c2fe218e051f..f2201f380c17 100644
@@ -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);

drivers/usb/host/xhci-pci.h new file +20 -0

new file mode 100644
index 000000000000..fc0cde231679
@@ -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 */

drivers/usb/host/xhci.h +1 -0

index fabbce1c542a..847d2021fc2c 100644
@@ -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);