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;
>
> Really? That's a lot simpler. But will it also fool gcc? That is,
> will it prevent gcc from optimizing away the !wIndex tests below?
>
This only fools ubsan, but it's still undefined behavior => checks
could be optimized away,
but it seems that current gcc(5.3.0) doesn't do this yet:
$ cat test.c
int a[10];
int test(int i) {
int *p = &a[i & 0xff - 1];
if (!i)
return 100;
else
return *p + 10;
}
$ gcc -O3 -c test.c
$ objdump -d test.o
0000000000000000 <test>:
0: 85 ff test %edi,%edi
2: b8 64 00 00 00 mov $0x64,%eax
7: 75 07 jne 10 <test+0x10>
9: f3 c3 repz retq
b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
10: 81 e7 fe 00 00 00 and $0xfe,%edi
16: 8b 04 bd 00 00 00 00 mov 0x0(,%rdi,4),%eax
1d: 83 c0 0a add $0xa,%eax
20: c3 retq
> How about this patch?
>
So it silences UBSAN, but still undefined.
I think it's up to you to decide - more code churn or undefined behavior.
> 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
> @@ -872,9 +872,10 @@ 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];
> + u32 __iomem *status_reg = ehci->regs->port_status +
> + ((wIndex & 0xff) - 1);
> + u32 __iomem *hostpc_reg = ehci->regs->hostpc +
> + ((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
