aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMathias Nyman2021-01-29 15:00:30 +0200
committerGreg Kroah-Hartman2021-01-29 14:16:50 +0100
commitc716e8a5fada15df495aa4891f4f22e51b95de11 (patch)
tree0d0382c82e4ce554f07d580ab9e4c89607b7c3bb
parent0353810a047ef0101af01665f60597d7f5cc6b02 (diff)
xhci: Check link TRBs when updating ring enqueue and dequeue pointers.
xhci driver relies on link TRBs existing in the correct places in TRB ring buffers shared with the host controller. The controller should not modify these link TRBs, but in theory a faulty xHC could do it. Add some basic sanity checks to avoid infinite loops in interrupt handler, or accessing unallocated memory outside a ring segment due to missing or misplaced link TRBs. Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Link: https://lore.kernel.org/r/20210129130044.206855-14-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/usb/host/xhci-ring.c37
1 files changed, 32 insertions, 5 deletions
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a078f465770f..2cbdc4537e73 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -151,10 +151,11 @@ static void next_trb(struct xhci_hcd *xhci,
/*
* See Cycle bit rules. SW is the consumer for the event ring only.
- * Don't make a ring full of link TRBs. That would be dumb and this would loop.
*/
void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
{
+ unsigned int link_trb_count = 0;
+
/* event ring doesn't have link trbs, check for last trb */
if (ring->type == TYPE_EVENT) {
if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
@@ -170,14 +171,23 @@ void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
/* All other rings have link trbs */
if (!trb_is_link(ring->dequeue)) {
- ring->dequeue++;
- ring->num_trbs_free++;
+ if (last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
+ xhci_warn(xhci, "Missing link TRB at end of segment\n");
+ } else {
+ ring->dequeue++;
+ ring->num_trbs_free++;
+ }
}
+
while (trb_is_link(ring->dequeue)) {
ring->deq_seg = ring->deq_seg->next;
ring->dequeue = ring->deq_seg->trbs;
- }
+ if (link_trb_count++ > ring->num_segs) {
+ xhci_warn(xhci, "Ring is an endless link TRB loop\n");
+ break;
+ }
+ }
out:
trace_xhci_inc_deq(ring);
@@ -186,7 +196,6 @@ out:
/*
* See Cycle bit rules. SW is the consumer for the event ring only.
- * Don't make a ring full of link TRBs. That would be dumb and this would loop.
*
* If we've just enqueued a TRB that is in the middle of a TD (meaning the
* chain bit is set), then set the chain bit in all the following link TRBs.
@@ -206,11 +215,18 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
{
u32 chain;
union xhci_trb *next;
+ unsigned int link_trb_count = 0;
chain = le32_to_cpu(ring->enqueue->generic.field[3]) & TRB_CHAIN;
/* If this is not event ring, there is one less usable TRB */
if (!trb_is_link(ring->enqueue))
ring->num_trbs_free--;
+
+ if (last_trb_on_seg(ring->enq_seg, ring->enqueue)) {
+ xhci_err(xhci, "Tried to move enqueue past ring segment\n");
+ return;
+ }
+
next = ++(ring->enqueue);
/* Update the dequeue pointer further if that was a link TRB */
@@ -247,6 +263,11 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring *ring,
ring->enq_seg = ring->enq_seg->next;
ring->enqueue = ring->enq_seg->trbs;
next = ring->enqueue;
+
+ if (link_trb_count++ > ring->num_segs) {
+ xhci_warn(xhci, "%s: Ring link TRB loop\n", __func__);
+ break;
+ }
}
trace_xhci_inc_enq(ring);
@@ -3035,6 +3056,12 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
return -EINVAL;
}
}
+
+ if (last_trb_on_seg(ep_ring->enq_seg, ep_ring->enqueue)) {
+ xhci_warn(xhci, "Missing link TRB at end of ring segment\n");
+ return -EINVAL;
+ }
+
return 0;
}