On Sat, 2 Apr 2016, Navin P.S wrote:
> >> --- a/drivers/usb/host/ehci-hub.c
> >> +++ b/drivers/usb/host/ehci-hub.c
> >> @@ -872,6 +872,10 @@ int ehci_hub_control(
> >> ) {
> >> struct ehci_hcd *ehci = hcd_to_ehci (hcd);
> >> int ports = HCS_N_PORTS (ehci->hcs_params);
> >> +
> >> + if (!wIndex || wIndex > ports)
> >> + return -EPIPE;
> >> +
> >
> > No, this is bad. There are cases where wIndex is allowed to be 0 or
> > larger than ports.
> >
>
>
> Ok, wIndex can be > ports.
>
> why is wIndex allowed to be 0 when we do in ehci_hub_control
>
>
> u32 __iomem *status_reg = &ehci->regs->port_status[
> (wIndex & 0xff) - 1];
> u32 __iomem *hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
You're asking the question backwards. wIndex is allowed to be 0
because the USB spec says so. You can't argue with that.
You should be asking why we initialize status_reg and hostpc_reg as
above.
> This is only valid when we have regs->port_status and regs->hostpc and
> 1 more into the actual array but gcc catches this.
No, it's always valid. The C spec might not agree with me, but the
kernel doesn't use standard C. It uses gcc, which is different from
the standard in quite a few ways.
> >> Feb 08 22:58:56 x kernel: UBSAN: Undefined behaviour in
> >> drivers/usb/host/ehci-hub.c:873:47
> >> Feb 08 22:58:56 x kernel: index -1 is out of range for type 'u32 [1]'
> >
> > UBSAN is wrong here. Even though the index is out of range, the
> > behavior is not undefined.
> >
>
> I was of the opinion that index is negative and the ptr is equal to
> array starting address ,negative indexes are undefined . So is the ptr
> + length of array. We can access the address for comparision but never
> its value as far as i could remember.
That's right. If you look more closely at the ehci_hub_control() code,
you'll see that we never dereference these pointers (or use them for
comparisons) when wIndex is 0.
> Can you please tell even though the index is out of range, why the
> behavior is defined ?
The behavior is defined because we don't use the pointers.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
