> From: Wang, Lin X
> 2013/12/16 David Laight <David.Laight@xxxxxxxxxx>:
> >> From: Of Wang, Lin X
> >> Sent: 15 December 2013 14:21
> >> To: Sarah Sharp (sarah.a.sharp@xxxxxxxxxxxxxxx)
> >> Cc: linux-usb@xxxxxxxxxxxxxxx
> >> Subject: [PATCH] xhci: fix array index out of the bounds in function last_trb() and
> >> last_trb_on_last_seg()
> >>
> >> In function last_trb() and last_trb_on_last_seg(), incorrect array index value 'TRBS_PER_SEGMENT'
> is
> >> used to determine the last element in an event ring segment, which lead to the out-of-bounds
> >> of index. But according to the current logic of event ring operation, this "bug" brings no problems
> >> and the driver works as desired. This patch only fix this array index out-of-bounds issue and there
> is
> >> no need no modify corresponding ring operation.
> >>
> >> Signed-off-by: Lin Wang <lin.x.wang@xxxxxxxxx>
> >> ---
> >> drivers/usb/host/xhci-ring.c | 10 +++++-----
> >> 1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> >> index d26cd94..3f93b07 100644
> >> --- a/drivers/usb/host/xhci-ring.c
> >> +++ b/drivers/usb/host/xhci-ring.c
> >> @@ -98,21 +98,21 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci, struct xhci_ring *ring,
> >> struct xhci_segment *seg, union xhci_trb *trb)
> >> {
> >> if (ring == xhci->event_ring)
> >> - return (trb == &seg->trbs[TRBS_PER_SEGMENT]) &&
> >> + return (trb == &seg->trbs[TRBS_PER_SEGMENT - 1]) &&
> >
> > Taking the address of the element beyond the end of an array is valid C.
>
> It's vaild in C doesn't mean that it‘s reasonable, besides,
> 'TRBS_PER_SEGMENT - 1' is widely used in current implementation to
> identily the last element in an array, using 'TRBS_PER_SEGMENT' here
> result in
> a lack of unity in style and easy to get lost.
There is a big difference between the 'event' ring (written by the xhci hardware)
and all the other rings (read by the hardware).
The last entry of the non-event rings is always a LINK trb (pointing to the
next ring segment) so must not be overwritten.
The event rings have a separate array that defines the segments and the hardware
writes real data into the last ring entry.
The operation of the event ring is different from the other rings and it was
(IMHO) a mistake to have used common functions for the two.
David
--
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
