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>

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a078f46..2cbdc45 100644 (file)
--- 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;
 }