"One Ring to rule them all, One Ring to find them,
One Ring to bring them all and in the darkness bind them"
- J. R. R. Tolkien
There is only one command ring for each xHCI host, and all commands flow
through that ring. However, when a tricksy little USB device fails to
respond to the Set Address command, the entire ring hangs.
If a Stop Endpoint command had been queued at the same time, the
watchdog timer will fire shortly after we start aborting the Set Address
command. The watchdog timer will assume the host is dying and kill it,
disconnecting all USB devices under the host. It's basically a USB DoS
attack.
The fix is for all commands to check whether they are the currently
executing command on the command ring when they timeout. If not, they
will wait on the command completion again, and the Stop Endpoint
watchdog timer will re-queue itself. If they are the currently
executing command, they will cancel themselves, or the Stop Endpoint
watchdog timer will assume the host is dying.
This patch should be backported to stable kernels as old as 3.0, that
contain the backported commit 6e4468b9a0793dfb53eb80d9fe52c739b13b27fd
"xHCI: cancel command after command timeout".
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Cc: Elric Fu <elricfu1@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
p.s. This is what happens when I listen to the "Howard Shore" Pandora
station while coding.
drivers/usb/host/xhci-ring.c | 14 ++++++++++
drivers/usb/host/xhci.c | 58 ++++++++++++++++++++++++++++++++++-------
drivers/usb/host/xhci.h | 1 +
3 files changed, 63 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0e32e19..306f1cc 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -925,6 +925,20 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
spin_unlock_irqrestore(&xhci->lock, flags);
return;
}
+ /*
+ * The Set Address command can take a long time to complete, and it will
+ * stall any other commands in the pipeline. If this command is not the
+ * currently executing command on the command ring, just restart the
+ * watchdog timer.
+ */
+ if (ep->stop_cmd_trb != xhci->cmd_ring->dequeue) {
+ ep->stop_cmds_pending++;
+ ep->stop_cmd_timer.expires = jiffies +
+ XHCI_STOP_EP_CMD_TIMEOUT * HZ;
+ add_timer(&ep->stop_cmd_timer);
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ return;
+ }
xhci_warn(xhci, "xHCI host not responding to stop endpoint command.\n");
xhci_warn(xhci, "Assuming host is dying, halting host.\n");
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4b6f677..b3ff5d9 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1539,6 +1539,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
ep->stop_cmd_timer.expires = jiffies +
XHCI_STOP_EP_CMD_TIMEOUT * HZ;
add_timer(&ep->stop_cmd_timer);
+ ep->stop_cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring);
xhci_queue_stop_endpoint(xhci, urb->dev->slot_id, ep_index, 0);
xhci_ring_cmd_db(xhci);
}
@@ -2536,6 +2537,42 @@ static int xhci_reserve_bandwidth(struct xhci_hcd *xhci,
return -ENOMEM;
}
+static int xhci_wait_for_command(struct xhci_hcd *xhci,
+ struct completion *cmd_completion,
+ union xhci_trb *cmd_trb,
+ char *command_name, unsigned long timeout)
+{
+ int timeleft;
+ unsigned long flags;
+
+ /* Wait for the configure endpoint command to complete */
+ do {
+ bool first_cmd;
+ timeleft = wait_for_completion_interruptible_timeout(
+ cmd_completion,
+ timeout);
+ if (timeleft != 0)
+ return timeleft;
+
+ /*
+ * Ok, the command timed out. Is it the first command in the
+ * list, or should we wait longer for other commands to finish
+ * or cancel? If we compare the dequeue, and it gets moved to
+ * our command just after we compare, the worst that will happen
+ * is that we wait once more through the timeout.
+ */
+ spin_lock_irqsave(&xhci->lock, flags);
+ first_cmd = (xhci->cmd_ring->dequeue == cmd_trb);
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ if (first_cmd)
+ return timeleft;
+
+ xhci_dbg(xhci, "Timeout while waiting for %s command, retry\n",
+ command_name);
+ } while (1);
+
+ return timeleft;
+}
/* Issue a configure endpoint command or evaluate context command
* and wait for it to finish.
@@ -2618,8 +2655,10 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci,
spin_unlock_irqrestore(&xhci->lock, flags);
/* Wait for the configure endpoint command to complete */
- timeleft = wait_for_completion_interruptible_timeout(
- cmd_completion,
+ timeleft = xhci_wait_for_command(xhci, cmd_completion, cmd_trb,
+ ctx_change == 0 ?
+ "configure endpoint" :
+ "evaluate context",
XHCI_CMD_DEFAULT_TIMEOUT);
if (timeleft <= 0) {
xhci_warn(xhci, "%s while waiting for %s command\n",
@@ -3404,9 +3443,9 @@ int xhci_discover_or_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
spin_unlock_irqrestore(&xhci->lock, flags);
/* Wait for the Reset Device command to finish */
- timeleft = wait_for_completion_interruptible_timeout(
- reset_device_cmd->completion,
- USB_CTRL_SET_TIMEOUT);
+ timeleft = xhci_wait_for_command(xhci, reset_device_cmd->completion,
+ reset_device_cmd->command_trb,
+ "reset device", USB_CTRL_SET_TIMEOUT);
if (timeleft <= 0) {
xhci_warn(xhci, "%s while waiting for reset device command\n",
timeleft == 0 ? "Timeout" : "Signal");
@@ -3592,8 +3631,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
- /* XXX: how much time for xHC slot assignment? */
- timeleft = wait_for_completion_interruptible_timeout(&xhci->addr_dev,
+ timeleft = xhci_wait_for_command(xhci, &xhci->addr_dev, cmd_trb,
+ "allocate device",
XHCI_CMD_DEFAULT_TIMEOUT);
if (timeleft <= 0) {
xhci_warn(xhci, "%s while waiting for a slot\n",
@@ -3710,9 +3749,8 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
- /* ctrl tx can take up to 5 sec; XXX: need more time for xHC? */
- timeleft = wait_for_completion_interruptible_timeout(&xhci->addr_dev,
- XHCI_CMD_DEFAULT_TIMEOUT);
+ timeleft = xhci_wait_for_command(xhci, &xhci->addr_dev, cmd_trb,
+ "set address", XHCI_CMD_DEFAULT_TIMEOUT);
/* FIXME: From section 4.3.4: "Software shall be responsible for timing
* the SetAddress() "recovery interval" required by USB and aborting the
* command on a timeout.
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2c510e4..f66ce67 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -844,6 +844,7 @@ struct xhci_virt_ep {
/* Watchdog timer for stop endpoint command to cancel URBs */
struct timer_list stop_cmd_timer;
int stop_cmds_pending;
+ union xhci_trb *stop_cmd_trb;
struct xhci_hcd *xhci;
/* Dequeue pointer and dequeue segment for a submitted Set TR Dequeue
* command. We'll need to update the ring's dequeue segment and dequeue
--
1.7.9
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
