From: Jack Pham <jackp@codeaurora.org>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: peter.chen@nxp.com, linux-imx@nxp.com, linux-usb@vger.kernel.org,
jun.li@nxp.com, joel@jms.id.au, mrana@codeaurora.org,
Chunfeng Yun <chunfeng.yun@mediatek.com>,
Thierry Reding <treding@nvidia.com>,
Jianguo Sun <sunjianguo1@huawei.com>,
stable@vger.kernel.org
Subject: [RFT,1/2] xhci: Fix leaking USB3 shared_hcd at xhci removal
Date: Fri, 28 Sep 2018 11:12:12 -0700 [thread overview]
Message-ID: <20180928181212.GD17520@jackp-linux.qualcomm.com> (raw)
On Thu, Sep 27, 2018 at 07:26:26PM +0300, Mathias Nyman wrote:
> 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 <joel@jms.id.au>
> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Jianguo Sun <sunjianguo1@huawei.com>
> Cc: <stable@vger.kernel.org>
> Reported-by: Jack Pham <jackp@codeaurora.org>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
> 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(-)
>
> diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
> index 27f0016..3c4abb5 100644
> --- a/drivers/usb/host/xhci-histb.c
> +++ b/drivers/usb/host/xhci-histb.c
> @@ -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);
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index 71d0d33..60987c7 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -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);
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 51dd8e0..92fd6b6 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -356,6 +356,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 */
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 94e9392..e5da8ce 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -359,14 +359,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);
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 4b463e5..b1cce98 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1240,6 +1240,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);
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 0420eef..c928dbb 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -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;
> }
Tested-by: Jack Pham <jackp@codeaurora.org>
next reply other threads:[~2018-09-28 18:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-28 18:12 Jack Pham [this message]
-- strict thread matches above, loose matches on Subject: below --
2018-09-28 3:35 [RFT,1/2] xhci: Fix leaking USB3 shared_hcd at xhci removal Peter Chen
2018-09-28 2:16 Chunfeng Yun
2018-09-27 16:26 Mathias Nyman
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=20180928181212.GD17520@jackp-linux.qualcomm.com \
--to=jackp@codeaurora.org \
--cc=chunfeng.yun@mediatek.com \
--cc=joel@jms.id.au \
--cc=jun.li@nxp.com \
--cc=linux-imx@nxp.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
--cc=mrana@codeaurora.org \
--cc=peter.chen@nxp.com \
--cc=stable@vger.kernel.org \
--cc=sunjianguo1@huawei.com \
--cc=treding@nvidia.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).