2016-05-18 22:28 GMT+03:00 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>: > On Wed, 18 May 2016, Andrey Ryabinin wrote: > >> 2016-05-18 19:09 GMT+03:00 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>: >> > On Wed, 18 May 2016, Andrey Ryabinin wrote: >> > >> >> 2016-05-18 17:40 GMT+03:00 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>: >> >> >> >> > All right, I'm getting very tired of all these bug reports. Besides, >> >> > Andrey has a point: Unless you're Linus, arguing against the C standard >> >> > is futile. (Even though the language dialect used in the kernel is not >> >> > standard C.) >> >> > >> >> > Does this patch make UBSAN happy? The runtime overhead is minimal. >> >> > >> >> >> >> It does. However, you could fool ubsan way more easy: >> >> u32 __iomem *hostpc_reg = ehci->regs->hostpc + >> >> (wIndex & 0xff) - 1; > > This probably should be considered to be a bug in UBSAN. It ought to > treat pointer addition the same as index addition. > It's more like a missing feature. UBSAN doesn't guarantee that every possible UB will be detected. >> >> So it silences UBSAN, but still undefined. >> I think it's up to you to decide - more code churn or undefined behavior. > > Well, I don't want the compiler to eliminate code that's necessary. > > On the other hand, it's not clear how much we need to worry about the > standard. After all, zero-length arrays are a GNU extension to C. > Since the array objects in question are defined like this: > > u32 port_status[0]; /* up to N_PORTS */ > > it's hard to guess what the compiler will think about out-of-bounds > pointer values. > > Maybe the best thing to do is eliminate the underflow while leaving the > calculation unchanged. What does UBSAN think about this? Does it > dislike -1 as an index value as much as it dislikes -1u? Type of doesn't change anything here. > Alan Stern > > > > Index: usb-4.x/drivers/usb/host/ehci-hub.c > =================================================================== > --- usb-4.x.orig/drivers/usb/host/ehci-hub.c > +++ usb-4.x/drivers/usb/host/ehci-hub.c > @@ -873,8 +873,9 @@ int ehci_hub_control( > struct ehci_hcd *ehci = hcd_to_ehci (hcd); > int ports = HCS_N_PORTS (ehci->hcs_params); > u32 __iomem *status_reg = &ehci->regs->port_status[ > - (wIndex & 0xff) - 1]; > - u32 __iomem *hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1]; > + ((int) wIndex & 0xff) - 1]; > + u32 __iomem *hostpc_reg = &ehci->regs->hostpc[ > + ((int) wIndex & 0xff) - 1]; > u32 temp, temp1, status; > unsigned long flags; > int retval = 0; > -- 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
