Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [Bug Report] drivers/usb/misc/sisusbvga: undefined result when left shift a possible negative value in sisusb_write_mem_bulk
@ 2020-05-20  3:51 Changming Liu
  2020-05-20  5:01 ` Greg KH
  0 siblings, 1 reply; 2+ messages in thread
From: Changming Liu @ 2020-05-20  3:51 UTC (permalink / raw)
  To: thomas, Greg KH; +Cc: linux-usb, Lu, Long, yaohway

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.

Due to the lack of knowledge of the interaction between this module and others, I'm not able to assess the severity of this problem. I wonder if it's worth a fix? If not, I would appreciate it if I can know why, this will help me understand linux and UB a lot!

Looking forward to your valuable response!

PS: I'm that guy who sent you a bug report and the patch was accepted 3 weeks ago, please allow me to express my appreciation again!

Changming Liu

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Bug Report] drivers/usb/misc/sisusbvga: undefined result when left shift a possible negative value in sisusb_write_mem_bulk
  2020-05-20  3:51 [Bug Report] drivers/usb/misc/sisusbvga: undefined result when left shift a possible negative value in sisusb_write_mem_bulk Changming Liu
@ 2020-05-20  5:01 ` Greg KH
  0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2020-05-20  5:01 UTC (permalink / raw)
  To: Changming Liu; +Cc: thomas, linux-usb, Lu, Long, yaohway

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.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20  3:51 [Bug Report] drivers/usb/misc/sisusbvga: undefined result when left shift a possible negative value in sisusb_write_mem_bulk Changming Liu
2020-05-20  5:01 ` Greg KH

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git