linux-kernel - Re: [syzbot] [input?] [usb?] UBSAN: shift-out-of-bounds in s32ton (2)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d21dcce3-88ba-416c-9d18-ea47855c48fc@rowland.harvard.edu>
Date: Wed, 16 Jul 2025 11:58:14 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: syzbot <syzbot+b63d677d63bcac06cf90@...kaller.appspotmail.com>
Cc: bentiss@...nel.org, jikos@...nel.org, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [input?] [usb?] UBSAN: shift-out-of-bounds in s32ton (2)

Benjamin:

On Wed, Jul 16, 2025 at 10:29:38AM -0400, Alan Stern wrote:
> On Tue, Jul 15, 2025 at 12:50:04PM -0700, syzbot wrote:
> > Hello,
> > 
> > syzbot has tested the proposed patch but the reproducer is still triggering an issue:
> > UBSAN: shift-out-of-bounds in s32ton
> > 
> > microsoft 0003:045E:07DA.0001: ignoring exceeding usage max
> > microsoft 0003:045E:07DA.0001: unsupported Resolution Multiplier 0
> > hid: s32ton: n 0 val 0 size 0x0
> > ------------[ cut here ]------------
> > UBSAN: shift-out-of-bounds in drivers/hid/hid-core.c:69:16
> > shift exponent 4294967295 is too large for 32-bit type '__s32' (aka 'int')
> 
> Benjamin:
> 
> Clearly there's going to be trouble when you try to convert a signed 
> 32-bit value to a 0-bit number!
> 
> My impression is that hid_parser_global() should reject Report Size or 
> Report Count items with a value of 0.  Such fields would be meaningless 
> in any case.  The routine checks for values that are too large, but not 
> for values that are too small.
> 
> Does this look like the right approach?
> 
> Alan Stern
> 
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git c2ca42f1
> 
>  drivers/hid/hid-core.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: usb-devel/drivers/hid/hid-core.c
> ===================================================================
> --- usb-devel.orig/drivers/hid/hid-core.c
> +++ usb-devel/drivers/hid/hid-core.c
> @@ -464,7 +464,8 @@ static int hid_parser_global(struct hid_
>  
>  	case HID_GLOBAL_ITEM_TAG_REPORT_SIZE:
>  		parser->global.report_size = item_udata(item);
> -		if (parser->global.report_size > 256) {
> +		if (parser->global.report_size > 256 ||
> +				parser->global.report_size == 0) {
>  			hid_err(parser->device, "invalid report_size %d\n",
>  					parser->global.report_size);
>  			return -1;
> @@ -473,7 +474,8 @@ static int hid_parser_global(struct hid_
>  
>  	case HID_GLOBAL_ITEM_TAG_REPORT_COUNT:
>  		parser->global.report_count = item_udata(item);
> -		if (parser->global.report_count > HID_MAX_USAGES) {
> +		if (parser->global.report_count > HID_MAX_USAGES ||
> +				parser->global.report_count == 0) {
>  			hid_err(parser->device, "invalid report_count %d\n",
>  					parser->global.report_count);
>  			return -1;

This patch didn't work; the error message never showed up in the kernel 
log.  Nevertheless, hidinput_change_resolution_multipliers() tried to 
create an output report with a field having size 0.

How can this to happen without hid_scan_report() or hid_open_report() 
running?  It shouldn't be possible to use a report before it has been 
checked for validity.

Alan Stern