> -----Original Message----- > From: Greg KH <greg@xxxxxxxxx> > Sent: Wednesday, May 20, 2020 12:47 PM > To: Changming Liu <liu.changm@xxxxxxxxxxxxxxxx> > Cc: thomas@xxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Lu, Long > <l.lu@xxxxxxxxxxxxxxxx>; yaohway@xxxxxxxxx > Subject: Re: [PATCH] USB: sisusbvga: Fix left shifting a possible negative value > > On Wed, May 20, 2020 at 04:37:50PM +0000, Changming Liu wrote: > > > > > > > -----Original Message----- > > > From: Greg KH <greg@xxxxxxxxx> > > > Sent: Wednesday, May 20, 2020 1:02 AM > > > To: Changming Liu <liu.changm@xxxxxxxxxxxxxxxx> > > > Cc: thomas@xxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Lu, Long > > > <l.lu@xxxxxxxxxxxxxxxx>; yaohway@xxxxxxxxx > > > Subject: Re: [Bug Report] drivers/usb/misc/sisusbvga: undefined > > > result when left shift a possible negative value in > > > sisusb_write_mem_bulk > > > > > > On Wed, May 20, 2020 at 03:51:04AM +0000, Changming Liu wrote: > > > > Hi Greg and Thomas, > > > > Greetings, I'm a first-year PhD student who is interested in the > > > > usage of > > > UBSan for linux. And after some experiments, I've found that in > > > drivers/usb/misc/sisusbvga/sisusb.c > > > > function sisusb_write_mem_bulk, there is an undefined behavior > > > > caused by > > > left shifting a possible negative number. > > > > > > > > More specifically, in the switch statement for case 3, after > > > > executing > > > copy_from_user, the the lower 3 bytes of char buf[4] are filled with > > > data from user space. > > > > And these 3 bytes are left shifted accordingly to form a 32bit > > > > unsigned > > > integer, swap32. > > > > > > > > The potential problem is, since the buf is declared as signed char > > > > buffer so > > > each byte might be a negative number while being left shifted. > > > According to the C standard, when the left-hand operand of the left > > > shift operator is a negative value, the result is undefined. So I > > > guess change the buf declaration to unsigned will help? Given that it's only > used here. > > > > > > Sounds like a good idea, patches are welcome to fix this. > > > > Hi greg, > > Thank you for this recognition! This means a lot to me. > > Here's the patch as we agreed. > > Please resend this in a normal format where we can properly review it. Sure, I'm so sorry for this inconvenience, I've sent you a separate patch , hope this works. > > But: > > > > > Best, > > Changming > > > > > > >From 14ae7c67ea3fb96ed6bea0bc9919f3c597308813 Mon Sep 17 00:00:00 > > >2001 > > From: Changming Liu <liu.changm@xxxxxxxxxxxxxxxx> > > Date: Wed, 20 May 2020 12:19:37 -0400 > > Subject: [PATCH] USB: sisusbvga: Fix left shifting a possible negative > > value > > > > the char buffer buf, accepts user data which might be negative value and the > content is left shifted to form an unsigned integer. > > Since left shifting a negative value is undefined behavior, thus > > change the char to u8 to fix this > > Properly line-wrap your changelog when you resend this. [Changming Liu] Got it, I believe in my patch, I properly line-wrapped the line, I hope the patch you received is well-formatted. Otherwise, I'll have to send through my gmail account. Sorry again for this inconvenience. Best, Changming
