Skip to contentSkip to conversation

xhci: Fix leaking USB3 shared_hcd at xhci removal

PATCH New Mathias Nyman 1 participant 1 message Nov 12, 2018

Subsystems (5)
USB Arm MediaTek Tegra architecture support USB xHCI driver +1 more subsystems
lore
Share & tools
lore

Mathias Nyman

author
#0 lore raw

PATCH 1/6 xhci: Fix leaking USB3 shared_hcd at xhci removal

Ensure that the shared_hcd pointer is valid when calling usb_put_hcd()

The shared_hcd is removed and freed in xhci by first calling usb_remove_hcd(xhci->shared_hcd), and later usb_put_hcd(xhci->shared_hcd)

Afer commit fe190ed0d602 ("xhci: Do not halt the host until both HCD have disconnected their devices.") the shared_hcd was never properly put as xhci->shared_hcd was set to NULL before usb_put_hcd(xhci->shared_hcd) was called.

shared_hcd (USB3) is removed before primary hcd (USB2). While removing the primary hcd we might need to handle xhci interrupts to cleanly remove last USB2 devices, therefore we need to set xhci->shared_hcd to NULL before removing the primary hcd to let xhci interrupt handler know shared_hcd is no longer available.

xhci-plat.c, xhci-histb.c and xhci-mtk first create both their hcd's before adding them. so to keep the correct reverse removal order use a temporary shared_hcd variable for them.
For more details see commit 4ac53087d6d4 ("usb: xhci: plat: Create both HCDs before adding them")

Fixes fe190ed0d602 ("xhci: Do not halt the host until both HCD have disconnected their devices.")
Cc Joel Stanley <[email protected]>
Cc Chunfeng Yun <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jianguo Sun <[email protected]>
Cc <[email protected]>
Reported-by Jack Pham <[email protected]>
Tested-by Jack Pham <[email protected]>
Tested-by Peter Chen <[email protected]>
Signed-off-by Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-histb.c | 6 ++++--
 drivers/usb/host/xhci-mtk.c   | 6 ++++--
 drivers/usb/host/xhci-pci.c   | 1 +
 drivers/usb/host/xhci-plat.c  | 6 ++++--
 drivers/usb/host/xhci-tegra.c | 1 +
 drivers/usb/host/xhci.c       | 2 --
 6 files changed, 14 insertions(+), 8 deletions(-)
drivers/usb/host/xhci-histb.c +4 -2
index 27f0016..3c4abb5 100644
@@ -325,14 +325,16 @@ static int xhci_histb_remove(struct platform_device *dev)
 	struct xhci_hcd_histb *histb = platform_get_drvdata(dev);
 	struct usb_hcd *hcd = histb->hcd;
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	struct usb_hcd *shared_hcd = xhci->shared_hcd;
 
 	xhci->xhc_state |= XHCI_STATE_REMOVING;
 
-	usb_remove_hcd(xhci->shared_hcd);
+	usb_remove_hcd(shared_hcd);
+	xhci->shared_hcd = NULL;
 	device_wakeup_disable(&dev->dev);
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(xhci->shared_hcd);
+	usb_put_hcd(shared_hcd);
 
 	xhci_histb_host_disable(histb);
 	usb_put_hcd(hcd);
drivers/usb/host/xhci-mtk.c +4 -2
index 71d0d33..60987c7 100644
@@ -590,12 +590,14 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	struct xhci_hcd_mtk *mtk = platform_get_drvdata(dev);
 	struct usb_hcd	*hcd = mtk->hcd;
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	struct usb_hcd  *shared_hcd = xhci->shared_hcd;
 
-	usb_remove_hcd(xhci->shared_hcd);
+	usb_remove_hcd(shared_hcd);
+	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(xhci->shared_hcd);
+	usb_put_hcd(shared_hcd);
 	usb_put_hcd(hcd);
 	xhci_mtk_sch_exit(mtk);
 	xhci_mtk_clks_disable(mtk);
drivers/usb/host/xhci-pci.c +1 -0
index 01c5705..1fb448c 100644
@@ -380,6 +380,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
 	if (xhci->shared_hcd) {
 		usb_remove_hcd(xhci->shared_hcd);
 		usb_put_hcd(xhci->shared_hcd);
+		xhci->shared_hcd = NULL;
 	}
 
 	/* Workaround for spurious wakeups at shutdown with HSW */
drivers/usb/host/xhci-plat.c +4 -2
index 32b5574..ef09cb0 100644
@@ -362,14 +362,16 @@ static int xhci_plat_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct clk *clk = xhci->clk;
 	struct clk *reg_clk = xhci->reg_clk;
+	struct usb_hcd *shared_hcd = xhci->shared_hcd;
 
 	xhci->xhc_state |= XHCI_STATE_REMOVING;
 
-	usb_remove_hcd(xhci->shared_hcd);
+	usb_remove_hcd(shared_hcd);
+	xhci->shared_hcd = NULL;
 	usb_phy_shutdown(hcd->usb_phy);
 
 	usb_remove_hcd(hcd);
-	usb_put_hcd(xhci->shared_hcd);
+	usb_put_hcd(shared_hcd);
 
 	clk_disable_unprepare(clk);
 	clk_disable_unprepare(reg_clk);
drivers/usb/host/xhci-tegra.c +1 -0
index 6b5db34..938ff06 100644
@@ -1303,6 +1303,7 @@ static int tegra_xusb_remove(struct platform_device *pdev)
 
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_put_hcd(xhci->shared_hcd);
+	xhci->shared_hcd = NULL;
 	usb_remove_hcd(tegra->hcd);
 	usb_put_hcd(tegra->hcd);
drivers/usb/host/xhci.c +0 -2
index 0420eef..c928dbb 100644
@@ -719,8 +719,6 @@ static void xhci_stop(struct usb_hcd *hcd)
 
 	/* Only halt host and free memory after both hcds are removed */
 	if (!usb_hcd_is_primary_hcd(hcd)) {
-		/* usb core will free this hcd shortly, unset pointer */
-		xhci->shared_hcd = NULL;
 		mutex_unlock(&xhci->mutex);
 		return;
 	}