Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: <gregkh@linuxfoundation.org>
Cc: <linux-usb@vger.kernel.org>,
	Bill Kuzeja <William.Kuzeja@stratus.com>,
	"# v4 . 17+" <stable@vger.kernel.org>,
	Torez Smith <torez@redhat.com>,
	Bill Kuzeja <william.kuzeja@stratus.com>,
	Mathias Nyman <mathias.nyman@linux.intel.com>
Subject: [PATCH 6/8] xhci: Prevent deadlock when xhci adapter breaks during init
Date: Fri,  4 Oct 2019 14:59:31 +0300
Message-ID: <1570190373-30684-7-git-send-email-mathias.nyman@linux.intel.com> (raw)
In-Reply-To: <1570190373-30684-1-git-send-email-mathias.nyman@linux.intel.com>

From: Bill Kuzeja <William.Kuzeja@stratus.com>

The system can hit a deadlock if an xhci adapter breaks while initializing.
The deadlock is between two threads: thread 1 is tearing down the
adapter and is stuck in usb_unlocked_disable_lpm waiting to lock the
hcd->handwidth_mutex. Thread 2 is holding this mutex (while still trying
to add a usb device), but is stuck in xhci_endpoint_reset waiting for a
stop or config command to complete. A reboot is required to resolve.

It turns out when calling xhci_queue_stop_endpoint and
xhci_queue_configure_endpoint in xhci_endpoint_reset, the return code is
not checked for errors. If the timing is right and the adapter dies just
before either of these commands get issued, we hang indefinitely waiting
for a completion on a command that didn't get issued.

This wasn't a problem before the following fix because we didn't send
commands in xhci_endpoint_reset:

commit f5249461b504 ("xhci: Clear the host side toggle manually when
    endpoint is soft reset")

With the patch I am submitting, a duration test which breaks adapters
during initialization (and which deadlocks with the standard kernel) runs
without issue.

Fixes: f5249461b504 ("xhci: Clear the host side toggle manually when
    endpoint is soft reset")
Cc: <stable@vger.kernel.org> # v4.17+
Cc: Torez Smith <torez@redhat.com>
Signed-off-by: Bill Kuzeja <william.kuzeja@stratus.com>
Signed-off-by: Torez Smith <torez@redhat.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fe8f9ff58015..8c068d0cc7c1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3095,6 +3095,7 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 	unsigned int ep_index;
 	unsigned long flags;
 	u32 ep_flag;
+	int err;
 
 	xhci = hcd_to_xhci(hcd);
 	if (!host_ep->hcpriv)
@@ -3154,7 +3155,17 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 		xhci_free_command(xhci, cfg_cmd);
 		goto cleanup;
 	}
-	xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id, ep_index, 0);
+
+	err = xhci_queue_stop_endpoint(xhci, stop_cmd, udev->slot_id,
+					ep_index, 0);
+	if (err < 0) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		xhci_free_command(xhci, cfg_cmd);
+		xhci_dbg(xhci, "%s: Failed to queue stop ep command, %d ",
+				__func__, err);
+		goto cleanup;
+	}
+
 	xhci_ring_cmd_db(xhci);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
@@ -3168,8 +3179,16 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
 					   ctrl_ctx, ep_flag, ep_flag);
 	xhci_endpoint_copy(xhci, cfg_cmd->in_ctx, vdev->out_ctx, ep_index);
 
-	xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
+	err = xhci_queue_configure_endpoint(xhci, cfg_cmd, cfg_cmd->in_ctx->dma,
 				      udev->slot_id, false);
+	if (err < 0) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		xhci_free_command(xhci, cfg_cmd);
+		xhci_dbg(xhci, "%s: Failed to queue config ep command, %d ",
+				__func__, err);
+		goto cleanup;
+	}
+
 	xhci_ring_cmd_db(xhci);
 	spin_unlock_irqrestore(&xhci->lock, flags);
 
-- 
2.7.4


  parent reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-04 11:59 [PATCH 0/8] xhci fixes for usb-linus Mathias Nyman
2019-10-04 11:59 ` [PATCH 1/8] xhci: Fix false warning message about wrong bounce buffer write length Mathias Nyman
2019-10-04 11:59 ` [PATCH 2/8] xhci: Prevent device initiated U1/U2 link pm if exit latency is too long Mathias Nyman
2019-10-04 11:59 ` [PATCH 3/8] xhci: Check all endpoints for LPM timeout Mathias Nyman
2019-10-04 11:59 ` [PATCH 4/8] xhci: Fix USB 3.1 capability detection on early xHCI 1.1 spec based hosts Mathias Nyman
2019-10-04 11:59 ` [PATCH 5/8] usb: xhci: wait for CNR controller not ready bit in xhci resume Mathias Nyman
2019-10-04 11:59 ` Mathias Nyman [this message]
2019-10-04 11:59 ` [PATCH 7/8] xhci: Increase STS_SAVE timeout in xhci_suspend() Mathias Nyman
     [not found]   ` <20191006120750.5334F2087E@mail.kernel.org>
2019-10-07 18:51     ` Kai-Heng Feng
2019-10-04 11:59 ` [PATCH 8/8] xhci: Fix NULL pointer dereference in xhci_clear_tt_buffer_complete() Mathias Nyman
2019-10-07 14:02   ` Johan Hovold
2019-10-08  8:15     ` Mathias Nyman
2019-10-11 12:47       ` Mathias Nyman
2019-10-11 12:58         ` [RFT PATCH] xhci: Fix use-after-free regression in xhci clear hub TT implementation Mathias Nyman
2019-10-14 10:16           ` Johan Hovold
2019-10-14 13:18             ` Mathias Nyman

Reply instructions:

You may reply publically 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=1570190373-30684-7-git-send-email-mathias.nyman@linux.intel.com \
    --to=mathias.nyman@linux.intel.com \
    --cc=William.Kuzeja@stratus.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torez@redhat.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

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git