Re: function ehci_hub_control in ehci-hub.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux