Hi Sarah,
David is right, this patch may lead to the last trb in an event ring unprocessed according to the current logic, you can reject this patch, although I think index out-of-bounds is reasonable.
If applying this patch, then corresponding function(inc_deq()) should be modified, maybe like the following way:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d26cd94..0dbaa56 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -185,7 +185,7 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
} else {
ring->dequeue++;
}
- } while (last_trb(xhci, ring, ring->deq_seg, ring->dequeue));
+ } while (last_trb(xhci, ring, ring->deq_seg, ring->dequeue) && ring->type != TYPE_EVENT);
addr = (unsigned long long) xhci_trb_virt_to_dma(ring->deq_seg, ring->dequeue);
}
Thanks & Br,
Lin
-----Original Message-----
From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx]
Sent: Tuesday, December 17, 2013 3:50 AM
To: David Laight
Cc: Lin; Wang, Lin X; linux-usb@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] xhci: fix array index out of the bounds in function last_trb() and last_trb_on_last_seg()
On Mon, Dec 16, 2013 at 03:20:28PM -0000, David Laight wrote:
> > I know all these difference clearly, inc_deq() is indeed a common
> > function for different rings, but lasr_trb() &
> > last_trb_on_last_seg() inside it use different condition to
> > determine the last trb in an event ring and an non-event ring; and
> > sorry, i still not find why last trb in an event ring skipped by H/W according to the current logic.
> >
> > Thanks!
> >
>
> Read the specs and the code.
State your objections clearly.
Based on my analysis, this patch will produce correct behavior with the event ring:
http://marc.info/?l=linux-usb&m=138697807827548&w=2
Am I missing something?
Sarah Sharp
--
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
