linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Changming Liu <liu.changm@northeastern.edu>
To: "thomas@winischhofer.net" <thomas@winischhofer.net>,
	Greg KH <greg@kroah.com>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"Lu, Long" <l.lu@northeastern.edu>,
	"yaohway@gmail.com" <yaohway@gmail.com>
Subject: [Bug Report] drivers/usb/misc/sisusbvga: undefined result when left shift a possible negative value in sisusb_write_mem_bulk
Date: Wed, 20 May 2020 03:51:04 +0000	[thread overview]
Message-ID: <BL0PR06MB4548D5526F5BFAD528039FECE5B60@BL0PR06MB4548.namprd06.prod.outlook.com> (raw)

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

             reply	other threads:[~2020-05-20  3:51 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-20  3:51 Changming Liu [this message]
2020-05-20  5:01 ` [Bug Report] drivers/usb/misc/sisusbvga: undefined result when left shift a possible negative value in sisusb_write_mem_bulk Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL0PR06MB4548D5526F5BFAD528039FECE5B60@BL0PR06MB4548.namprd06.prod.outlook.com \
    --to=liu.changm@northeastern.edu \
    --cc=greg@kroah.com \
    --cc=l.lu@northeastern.edu \
    --cc=linux-usb@vger.kernel.org \
    --cc=thomas@winischhofer.net \
    --cc=yaohway@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).