On Fri, Apr 1, 2016 at 9:19 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 1 Apr 2016, Navin P.S wrote: > >> On Fri, Apr 1, 2016 at 8:00 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: >> > On Fri, 1 Apr 2016, Navin P.S wrote: >> > >> >> Hi, >> >> >> >> I was looking at the bug https://bugzilla.kernel.org/show_bug.cgi?id=112171 >> >> which says >> >> >> >> 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]' >> >> >> In case of wIndex can we return -EPIPE as in case of error in >> ehci-hub.c or do we have to return some other error code ? > > If wIndex is wrong, you should always return -EPIPE. > >> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c >> index ffc9029..50194af 100644 >> --- 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]; This is only valid when we have regs->port_status and regs->hostpc and 1 more into the actual array but gcc catches this. >> In the below call trace we get wIndex as 0 due to >> usb_set_configuration calling it with 0. Please see below. >> >> 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. Can you please tell even though the index is out of range, why the behavior is defined ? This doesn't give an error which is correct since ptr is always within arr. /* gcc ubsan.c -fsanitize-undefined-trap-on-error -fsanitize=address -fsanitize=undefined */ #include<stdio.h> int main() { int arr[10]={0}; int i; int *ptr=arr; arr[0]=1; arr[1]=2; int idx=0; idx--; ptr++; printf("%d\n",arr[0]); ptr[idx]=3; printf("%d\n",ptr[idx]); idx=10; ptr=arr; ptr--; printf("%d\n",ptr[idx]); } -- 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
