linux-kernel - [patch 3/3] USB: xHCI: prevent infinite loop when processing MSE event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 19 Sep 2011 16:05:12 -0700
From:	Greg KH <gregkh@...e.de>
To:	torvalds@...ux-foundation.org
Cc:	Randy Dunlap <rdunlap@...otime.net>
Subject: [patch 3/3] USB: xHCI: prevent infinite loop when processing MSE event

From: Andiry Xu <andiry.xu@....com>

When a xHC host is unable to handle isochronous transfer in the interval,
it reports a Missed Service Error event and skips some tds.

Currently xhci driver handles MSE event in the following ways:

1. When encounter a MSE event, set ep->skip flag, update event ring dequeue
pointer and return.
2. When encounter the next event on this ep, the driver will run the do-while
loop, fetch td from ep's td_list to find the td corresponding to this event.
All tds missed are marked as short transfer(-EXDEV).

The do-while loop will end in two ways:
1. If the td pointed by the event trb is found;
2. If the ep ring's td_list is empty.

However, if a buggy HW reports some unpredicted event (for example, an overrun
event following a MSE event while the ep ring is actually not empty), the
driver will never find the td, and it will loop until the td_list is empty.

Unfortunately, the spinlock is dropped when give back a urb in the do-while
loop. During the spinlock released period, the class driver may still
submit urbs and add tds to the td_list. This may cause disaster, since the
td_list will never be empty and the loop never ends, and the system hangs.

To fix this, count the number of TDs on the ep ring before skipping TDs, and
quit the loop when skipped that number of tds. This guarantees the do-while
loop will end after certain number of cycles, and driver will not be trapped
in an infinite loop.

Signed-off-by: Andiry Xu <andiry.xu@....com>
Signed-off-by: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>

---
 drivers/usb/host/xhci-ring.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1934,8 +1934,10 @@ static int handle_tx_event(struct xhci_h
 	int status = -EINPROGRESS;
 	struct urb_priv *urb_priv;
 	struct xhci_ep_ctx *ep_ctx;
+	struct list_head *tmp;
 	u32 trb_comp_code;
 	int ret = 0;
+	int td_num = 0;

 	slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
 	xdev = xhci->devs[slot_id];
@@ -1957,6 +1959,12 @@ static int handle_tx_event(struct xhci_h
 		return -ENODEV;
 	}

+	/* Count current td numbers if ep->skip is set */
+	if (ep->skip) {
+		list_for_each(tmp, &ep_ring->td_list)
+			td_num++;
+	}
+
 	event_dma = le64_to_cpu(event->buffer);
 	trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
 	/* Look for common error cases */
@@ -2068,7 +2076,18 @@ static int handle_tx_event(struct xhci_h
 			goto cleanup;
 		}

+		/* We've skipped all the TDs on the ep ring when ep->skip set */
+		if (ep->skip && td_num == 0) {
+			ep->skip = false;
+			xhci_dbg(xhci, "All tds on the ep_ring skipped. "
+						"Clear skip flag.\n");
+			ret = 0;
+			goto cleanup;
+		}
+
 		td = list_entry(ep_ring->td_list.next, struct xhci_td, td_list);
+		if (ep->skip)
+			td_num--;

 		/* Is this a TRB in the currently executing TD? */
 		event_seg = trb_in_td(ep_ring->deq_seg, ep_ring->dequeue,

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/