linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* UBSAN: shift-out-of-bounds in dummy_hub_control
@ 2020-12-25 20:05 syzbot
  2020-12-29 16:33 ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: syzbot @ 2020-12-25 20:05 UTC (permalink / raw)
  To: andreyknvl, andreyknvl, balbi, gregkh, gustavoars, linux-kernel,
	linux-usb, stern, syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    e37b12e4 Merge tag 'for-linus-5.11-ofs1' of git://git.kern..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17429937500000
kernel config:  https://syzkaller.appspot.com/x/.config?x=98408202fed1f636
dashboard link: https://syzkaller.appspot.com/bug?extid=5925509f78293baa7331
compiler:       gcc (GCC) 10.1.0-syz 20200507
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1781fc5b500000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=157cd123500000

The issue was bisected to:

commit 8442b02bf3c6770e0d7e7ea17be36c30e95987b6
Author: Andrey Konovalov <andreyknvl@google.com>
Date:   Mon Oct 21 14:20:58 2019 +0000

    USB: dummy-hcd: increase max number of devices to 32

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1631d0db500000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=1531d0db500000
console output: https://syzkaller.appspot.com/x/log.txt?x=1131d0db500000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+5925509f78293baa7331@syzkaller.appspotmail.com
Fixes: 8442b02bf3c6 ("USB: dummy-hcd: increase max number of devices to 32")

================================================================================
UBSAN: shift-out-of-bounds in drivers/usb/gadget/udc/dummy_hcd.c:2293:33
shift exponent 257 is too large for 32-bit type 'int'
CPU: 0 PID: 8526 Comm: syz-executor949 Not tainted 5.10.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:79 [inline]
 dump_stack+0x107/0x163 lib/dump_stack.c:120
 ubsan_epilogue+0xb/0x5a lib/ubsan.c:148
 __ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:395
 dummy_hub_control.cold+0x1a/0xbc drivers/usb/gadget/udc/dummy_hcd.c:2293
 rh_call_control drivers/usb/core/hcd.c:683 [inline]
 rh_urb_enqueue drivers/usb/core/hcd.c:841 [inline]
 usb_hcd_submit_urb+0xcaa/0x22d0 drivers/usb/core/hcd.c:1544
 usb_submit_urb+0x6e4/0x1560 drivers/usb/core/urb.c:585
 usb_start_wait_urb+0x101/0x4c0 drivers/usb/core/message.c:58
 usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
 usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
 do_proc_control+0x4cb/0x9c0 drivers/usb/core/devio.c:1165
 proc_control drivers/usb/core/devio.c:1191 [inline]
 usbdev_do_ioctl drivers/usb/core/devio.c:2535 [inline]
 usbdev_ioctl+0x12c1/0x3b20 drivers/usb/core/devio.c:2708
 vfs_ioctl fs/ioctl.c:48 [inline]
 __do_sys_ioctl fs/ioctl.c:753 [inline]
 __se_sys_ioctl fs/ioctl.c:739 [inline]
 __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x443f29
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb d7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc10df4328 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004002e0 RCX: 0000000000443f29
RDX: 0000000020000000 RSI: 00000000c0185500 RDI: 0000000000000003
RBP: 00000000006ce018 R08: 0000000000000000 R09: 00000000004002e0
R10: 000000000000000f R11: 0000000000000246 R12: 0000000000401bb0
R13: 0000000000401c40 R14: 0000000000000000 R15: 0000000000000000
================================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

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

* Re: UBSAN: shift-out-of-bounds in dummy_hub_control
  2020-12-25 20:05 UBSAN: shift-out-of-bounds in dummy_hub_control syzbot
@ 2020-12-29 16:33 ` Alan Stern
  2020-12-29 16:33   ` syzbot
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2020-12-29 16:33 UTC (permalink / raw)
  To: syzbot
  Cc: andreyknvl, andreyknvl, balbi, gregkh, gustavoars, linux-kernel,
	linux-usb, syzkaller-bugs

On Fri, Dec 25, 2020 at 12:05:22PM -0800, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    e37b12e4 Merge tag 'for-linus-5.11-ofs1' of git://git.kern..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=17429937500000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=98408202fed1f636
> dashboard link: https://syzkaller.appspot.com/bug?extid=5925509f78293baa7331
> compiler:       gcc (GCC) 10.1.0-syz 20200507
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1781fc5b500000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=157cd123500000
> 
> The issue was bisected to:
> 
> commit 8442b02bf3c6770e0d7e7ea17be36c30e95987b6
> Author: Andrey Konovalov <andreyknvl@google.com>
> Date:   Mon Oct 21 14:20:58 2019 +0000
> 
>     USB: dummy-hcd: increase max number of devices to 32
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1631d0db500000
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=1531d0db500000
> console output: https://syzkaller.appspot.com/x/log.txt?x=1131d0db500000
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+5925509f78293baa7331@syzkaller.appspotmail.com
> Fixes: 8442b02bf3c6 ("USB: dummy-hcd: increase max number of devices to 32")
> 
> ================================================================================
> UBSAN: shift-out-of-bounds in drivers/usb/gadget/udc/dummy_hcd.c:2293:33
> shift exponent 257 is too large for 32-bit type 'int'
> CPU: 0 PID: 8526 Comm: syz-executor949 Not tainted 5.10.0-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:79 [inline]
>  dump_stack+0x107/0x163 lib/dump_stack.c:120
>  ubsan_epilogue+0xb/0x5a lib/ubsan.c:148
>  __ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:395
>  dummy_hub_control.cold+0x1a/0xbc drivers/usb/gadget/udc/dummy_hcd.c:2293
>  rh_call_control drivers/usb/core/hcd.c:683 [inline]
>  rh_urb_enqueue drivers/usb/core/hcd.c:841 [inline]
>  usb_hcd_submit_urb+0xcaa/0x22d0 drivers/usb/core/hcd.c:1544
>  usb_submit_urb+0x6e4/0x1560 drivers/usb/core/urb.c:585
>  usb_start_wait_urb+0x101/0x4c0 drivers/usb/core/message.c:58
>  usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
>  usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
>  do_proc_control+0x4cb/0x9c0 drivers/usb/core/devio.c:1165
>  proc_control drivers/usb/core/devio.c:1191 [inline]
>  usbdev_do_ioctl drivers/usb/core/devio.c:2535 [inline]
>  usbdev_ioctl+0x12c1/0x3b20 drivers/usb/core/devio.c:2708
>  vfs_ioctl fs/ioctl.c:48 [inline]
>  __do_sys_ioctl fs/ioctl.c:753 [inline]
>  __se_sys_ioctl fs/ioctl.c:739 [inline]
>  __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
>  do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x443f29
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb d7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffc10df4328 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00000000004002e0 RCX: 0000000000443f29
> RDX: 0000000020000000 RSI: 00000000c0185500 RDI: 0000000000000003
> RBP: 00000000006ce018 R08: 0000000000000000 R09: 00000000004002e0
> R10: 000000000000000f R11: 0000000000000246 R12: 0000000000401bb0
> R13: 0000000000401c40 R14: 0000000000000000 R15: 0000000000000000
> ================================================================================

The cause is pretty obvious.  dummy-hcd assumes that requests sent to 
the root hub are always valid, which isn't always true when they come 
from userspace.

Alan Stern

#syz test: upstream e37b12e4

Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2114,9 +2114,21 @@ static int dummy_hub_control(
 				dum_hcd->port_status &= ~USB_PORT_STAT_POWER;
 			set_link_state(dum_hcd);
 			break;
-		default:
+		case USB_PORT_FEAT_ENABLE:
+		case USB_PORT_FEAT_C_ENABLE:
+		case USB_PORT_FEAT_C_SUSPEND:
+			/* Not allowed for USB-3 */
+			if (hcd->speed == HCD_USB3)
+				goto error;
+			fallthrough;
+		case USB_PORT_FEAT_C_CONNECTION:
+		case USB_PORT_FEAT_C_RESET:
 			dum_hcd->port_status &= ~(1 << wValue);
 			set_link_state(dum_hcd);
+			break;
+		default:
+		/* Disallow INDICATOR and C_OVER_CURRENT */
+			goto error;
 		}
 		break;
 	case GetHubDescriptor:
@@ -2277,18 +2289,17 @@ static int dummy_hub_control(
 			 */
 			dum_hcd->re_timeout = jiffies + msecs_to_jiffies(50);
 			fallthrough;
+		case USB_PORT_FEAT_C_CONNECTION:
+		case USB_PORT_FEAT_C_RESET:
+		case USB_PORT_FEAT_C_ENABLE:
+		case USB_PORT_FEAT_C_SUSPEND:
+			/* Not allowed for USB-3, and ignored for USB-2 */
+			if (hcd->speed == HCD_USB3)
+				goto error;
+			break;
 		default:
-			if (hcd->speed == HCD_USB3) {
-				if ((dum_hcd->port_status &
-				     USB_SS_PORT_STAT_POWER) != 0) {
-					dum_hcd->port_status |= (1 << wValue);
-				}
-			} else
-				if ((dum_hcd->port_status &
-				     USB_PORT_STAT_POWER) != 0) {
-					dum_hcd->port_status |= (1 << wValue);
-				}
-			set_link_state(dum_hcd);
+		/* Disallow TEST, INDICATOR, and C_OVER_CURRENT */
+			goto error;
 		}
 		break;
 	case GetPortErrorCount:

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

* Re: Re: UBSAN: shift-out-of-bounds in dummy_hub_control
  2020-12-29 16:33 ` Alan Stern
@ 2020-12-29 16:33   ` syzbot
  2020-12-29 16:43     ` Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: syzbot @ 2020-12-29 16:33 UTC (permalink / raw)
  To: Alan Stern
  Cc: andreyknvl, andreyknvl, balbi, gregkh, gustavoars, linux-kernel,
	linux-usb, stern, syzkaller-bugs

> On Fri, Dec 25, 2020 at 12:05:22PM -0800, syzbot wrote:
>> Hello,
>> 
>> syzbot found the following issue on:
>> 
>> HEAD commit:    e37b12e4 Merge tag 'for-linus-5.11-ofs1' of git://git.kern..
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=17429937500000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=98408202fed1f636
>> dashboard link: https://syzkaller.appspot.com/bug?extid=5925509f78293baa7331
>> compiler:       gcc (GCC) 10.1.0-syz 20200507
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1781fc5b500000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=157cd123500000
>> 
>> The issue was bisected to:
>> 
>> commit 8442b02bf3c6770e0d7e7ea17be36c30e95987b6
>> Author: Andrey Konovalov <andreyknvl@google.com>
>> Date:   Mon Oct 21 14:20:58 2019 +0000
>> 
>>     USB: dummy-hcd: increase max number of devices to 32
>> 
>> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1631d0db500000
>> final oops:     https://syzkaller.appspot.com/x/report.txt?x=1531d0db500000
>> console output: https://syzkaller.appspot.com/x/log.txt?x=1131d0db500000
>> 
>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>> Reported-by: syzbot+5925509f78293baa7331@syzkaller.appspotmail.com
>> Fixes: 8442b02bf3c6 ("USB: dummy-hcd: increase max number of devices to 32")
>> 
>> ================================================================================
>> UBSAN: shift-out-of-bounds in drivers/usb/gadget/udc/dummy_hcd.c:2293:33
>> shift exponent 257 is too large for 32-bit type 'int'
>> CPU: 0 PID: 8526 Comm: syz-executor949 Not tainted 5.10.0-syzkaller #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> Call Trace:
>>  __dump_stack lib/dump_stack.c:79 [inline]
>>  dump_stack+0x107/0x163 lib/dump_stack.c:120
>>  ubsan_epilogue+0xb/0x5a lib/ubsan.c:148
>>  __ubsan_handle_shift_out_of_bounds.cold+0xb1/0x181 lib/ubsan.c:395
>>  dummy_hub_control.cold+0x1a/0xbc drivers/usb/gadget/udc/dummy_hcd.c:2293
>>  rh_call_control drivers/usb/core/hcd.c:683 [inline]
>>  rh_urb_enqueue drivers/usb/core/hcd.c:841 [inline]
>>  usb_hcd_submit_urb+0xcaa/0x22d0 drivers/usb/core/hcd.c:1544
>>  usb_submit_urb+0x6e4/0x1560 drivers/usb/core/urb.c:585
>>  usb_start_wait_urb+0x101/0x4c0 drivers/usb/core/message.c:58
>>  usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
>>  usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
>>  do_proc_control+0x4cb/0x9c0 drivers/usb/core/devio.c:1165
>>  proc_control drivers/usb/core/devio.c:1191 [inline]
>>  usbdev_do_ioctl drivers/usb/core/devio.c:2535 [inline]
>>  usbdev_ioctl+0x12c1/0x3b20 drivers/usb/core/devio.c:2708
>>  vfs_ioctl fs/ioctl.c:48 [inline]
>>  __do_sys_ioctl fs/ioctl.c:753 [inline]
>>  __se_sys_ioctl fs/ioctl.c:739 [inline]
>>  __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:739
>>  do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
>>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> RIP: 0033:0x443f29
>> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb d7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
>> RSP: 002b:00007ffc10df4328 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
>> RAX: ffffffffffffffda RBX: 00000000004002e0 RCX: 0000000000443f29
>> RDX: 0000000020000000 RSI: 00000000c0185500 RDI: 0000000000000003
>> RBP: 00000000006ce018 R08: 0000000000000000 R09: 00000000004002e0
>> R10: 000000000000000f R11: 0000000000000246 R12: 0000000000401bb0
>> R13: 0000000000401c40 R14: 0000000000000000 R15: 0000000000000000
>> ================================================================================
>
> The cause is pretty obvious.  dummy-hcd assumes that requests sent to 
> the root hub are always valid, which isn't always true when they come 
> from userspace.
>
> Alan Stern
>
> #syz test: upstream e37b12e4

"upstream" does not look like a valid git repo address.

>
> Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -2114,9 +2114,21 @@ static int dummy_hub_control(
>  				dum_hcd->port_status &= ~USB_PORT_STAT_POWER;
>  			set_link_state(dum_hcd);
>  			break;
> -		default:
> +		case USB_PORT_FEAT_ENABLE:
> +		case USB_PORT_FEAT_C_ENABLE:
> +		case USB_PORT_FEAT_C_SUSPEND:
> +			/* Not allowed for USB-3 */
> +			if (hcd->speed == HCD_USB3)
> +				goto error;
> +			fallthrough;
> +		case USB_PORT_FEAT_C_CONNECTION:
> +		case USB_PORT_FEAT_C_RESET:
>  			dum_hcd->port_status &= ~(1 << wValue);
>  			set_link_state(dum_hcd);
> +			break;
> +		default:
> +		/* Disallow INDICATOR and C_OVER_CURRENT */
> +			goto error;
>  		}
>  		break;
>  	case GetHubDescriptor:
> @@ -2277,18 +2289,17 @@ static int dummy_hub_control(
>  			 */
>  			dum_hcd->re_timeout = jiffies + msecs_to_jiffies(50);
>  			fallthrough;
> +		case USB_PORT_FEAT_C_CONNECTION:
> +		case USB_PORT_FEAT_C_RESET:
> +		case USB_PORT_FEAT_C_ENABLE:
> +		case USB_PORT_FEAT_C_SUSPEND:
> +			/* Not allowed for USB-3, and ignored for USB-2 */
> +			if (hcd->speed == HCD_USB3)
> +				goto error;
> +			break;
>  		default:
> -			if (hcd->speed == HCD_USB3) {
> -				if ((dum_hcd->port_status &
> -				     USB_SS_PORT_STAT_POWER) != 0) {
> -					dum_hcd->port_status |= (1 << wValue);
> -				}
> -			} else
> -				if ((dum_hcd->port_status &
> -				     USB_PORT_STAT_POWER) != 0) {
> -					dum_hcd->port_status |= (1 << wValue);
> -				}
> -			set_link_state(dum_hcd);
> +		/* Disallow TEST, INDICATOR, and C_OVER_CURRENT */
> +			goto error;
>  		}
>  		break;
>  	case GetPortErrorCount:

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

* Re: Re: UBSAN: shift-out-of-bounds in dummy_hub_control
  2020-12-29 16:33   ` syzbot
@ 2020-12-29 16:43     ` Alan Stern
  2020-12-29 17:00       ` syzbot
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2020-12-29 16:43 UTC (permalink / raw)
  To: syzbot
  Cc: andreyknvl, andreyknvl, balbi, gregkh, gustavoars, linux-kernel,
	linux-usb, syzkaller-bugs

On Tue, Dec 29, 2020 at 08:33:39AM -0800, syzbot wrote:
> > #syz test: upstream e37b12e4
>
> "upstream" does not look like a valid git repo address.

I thought syzbot had been changed to recognize "upstream" as a valid
repo name.

Alan Stern

#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e37b12e4

Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2114,9 +2114,21 @@ static int dummy_hub_control(
 				dum_hcd->port_status &= ~USB_PORT_STAT_POWER;
 			set_link_state(dum_hcd);
 			break;
-		default:
+		case USB_PORT_FEAT_ENABLE:
+		case USB_PORT_FEAT_C_ENABLE:
+		case USB_PORT_FEAT_C_SUSPEND:
+			/* Not allowed for USB-3 */
+			if (hcd->speed == HCD_USB3)
+				goto error;
+			fallthrough;
+		case USB_PORT_FEAT_C_CONNECTION:
+		case USB_PORT_FEAT_C_RESET:
 			dum_hcd->port_status &= ~(1 << wValue);
 			set_link_state(dum_hcd);
+			break;
+		default:
+		/* Disallow INDICATOR and C_OVER_CURRENT */
+			goto error;
 		}
 		break;
 	case GetHubDescriptor:
@@ -2277,18 +2289,17 @@ static int dummy_hub_control(
 			 */
 			dum_hcd->re_timeout = jiffies + msecs_to_jiffies(50);
 			fallthrough;
+		case USB_PORT_FEAT_C_CONNECTION:
+		case USB_PORT_FEAT_C_RESET:
+		case USB_PORT_FEAT_C_ENABLE:
+		case USB_PORT_FEAT_C_SUSPEND:
+			/* Not allowed for USB-3, and ignored for USB-2 */
+			if (hcd->speed == HCD_USB3)
+				goto error;
+			break;
 		default:
-			if (hcd->speed == HCD_USB3) {
-				if ((dum_hcd->port_status &
-				     USB_SS_PORT_STAT_POWER) != 0) {
-					dum_hcd->port_status |= (1 << wValue);
-				}
-			} else
-				if ((dum_hcd->port_status &
-				     USB_PORT_STAT_POWER) != 0) {
-					dum_hcd->port_status |= (1 << wValue);
-				}
-			set_link_state(dum_hcd);
+		/* Disallow TEST, INDICATOR, and C_OVER_CURRENT */
+			goto error;
 		}
 		break;
 	case GetPortErrorCount:


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

* Re: UBSAN: shift-out-of-bounds in dummy_hub_control
  2020-12-29 16:43     ` Alan Stern
@ 2020-12-29 17:00       ` syzbot
  2020-12-30 16:20         ` [PATCH] USB: Gadget: dummy-hcd: Fix shift-out-of-bounds bug Alan Stern
  0 siblings, 1 reply; 6+ messages in thread
From: syzbot @ 2020-12-29 17:00 UTC (permalink / raw)
  To: andreyknvl, andreyknvl, balbi, gregkh, gustavoars, linux-kernel,
	linux-usb, stern, syzkaller-bugs

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+5925509f78293baa7331@syzkaller.appspotmail.com

Tested on:

commit:         e37b12e4 Merge tag 'for-linus-5.11-ofs1' of git://git.kern..
git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel config:  https://syzkaller.appspot.com/x/.config?x=98408202fed1f636
dashboard link: https://syzkaller.appspot.com/bug?extid=5925509f78293baa7331
compiler:       gcc (GCC) 10.1.0-syz 20200507
patch:          https://syzkaller.appspot.com/x/patch.diff?x=16f0bc70d00000

Note: testing is done by a robot and is best-effort only.

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

* [PATCH] USB: Gadget: dummy-hcd: Fix shift-out-of-bounds bug
  2020-12-29 17:00       ` syzbot
@ 2020-12-30 16:20         ` Alan Stern
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Stern @ 2020-12-30 16:20 UTC (permalink / raw)
  To: Greg KH, Felipe Balbi; +Cc: gustavoars, USB mailing list, syzkaller-bugs

The dummy-hcd driver was written under the assumption that all the
parameters in URBs sent to its root hub would be valid.  With URBs
sent from userspace via usbfs, that assumption can be violated.

In particular, the driver doesn't fully check the port-feature values
stored in the wValue entry of Clear-Port-Feature and Set-Port-Feature
requests.  Values that are too large can cause the driver to perform
an invalid left shift of more than 32 bits.  Ironically, two of those
left shifts are unnecessary, because they implement Set-Port-Feature
requests that hubs are not required to support, according to section
11.24.2.13 of the USB-2.0 spec.

This patch adds the appropriate checks for the port feature selector
values and removes the unnecessary feature settings.  It also rejects
requests to set the TEST feature or to set or clear the INDICATOR and
C_OVERCURRENT features, as none of these are relevant to dummy-hcd's
root-hub emulation.

Reported-and-tested-by: syzbot+5925509f78293baa7331@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: <stable@vger.kernel.org>

---


[as1951]


 drivers/usb/gadget/udc/dummy_hcd.c |   35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2114,9 +2114,21 @@ static int dummy_hub_control(
 				dum_hcd->port_status &= ~USB_PORT_STAT_POWER;
 			set_link_state(dum_hcd);
 			break;
-		default:
+		case USB_PORT_FEAT_ENABLE:
+		case USB_PORT_FEAT_C_ENABLE:
+		case USB_PORT_FEAT_C_SUSPEND:
+			/* Not allowed for USB-3 */
+			if (hcd->speed == HCD_USB3)
+				goto error;
+			fallthrough;
+		case USB_PORT_FEAT_C_CONNECTION:
+		case USB_PORT_FEAT_C_RESET:
 			dum_hcd->port_status &= ~(1 << wValue);
 			set_link_state(dum_hcd);
+			break;
+		default:
+		/* Disallow INDICATOR and C_OVER_CURRENT */
+			goto error;
 		}
 		break;
 	case GetHubDescriptor:
@@ -2277,18 +2289,17 @@ static int dummy_hub_control(
 			 */
 			dum_hcd->re_timeout = jiffies + msecs_to_jiffies(50);
 			fallthrough;
+		case USB_PORT_FEAT_C_CONNECTION:
+		case USB_PORT_FEAT_C_RESET:
+		case USB_PORT_FEAT_C_ENABLE:
+		case USB_PORT_FEAT_C_SUSPEND:
+			/* Not allowed for USB-3, and ignored for USB-2 */
+			if (hcd->speed == HCD_USB3)
+				goto error;
+			break;
 		default:
-			if (hcd->speed == HCD_USB3) {
-				if ((dum_hcd->port_status &
-				     USB_SS_PORT_STAT_POWER) != 0) {
-					dum_hcd->port_status |= (1 << wValue);
-				}
-			} else
-				if ((dum_hcd->port_status &
-				     USB_PORT_STAT_POWER) != 0) {
-					dum_hcd->port_status |= (1 << wValue);
-				}
-			set_link_state(dum_hcd);
+		/* Disallow TEST, INDICATOR, and C_OVER_CURRENT */
+			goto error;
 		}
 		break;
 	case GetPortErrorCount:

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

end of thread, other threads:[~2020-12-30 16:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-25 20:05 UBSAN: shift-out-of-bounds in dummy_hub_control syzbot
2020-12-29 16:33 ` Alan Stern
2020-12-29 16:33   ` syzbot
2020-12-29 16:43     ` Alan Stern
2020-12-29 17:00       ` syzbot
2020-12-30 16:20         ` [PATCH] USB: Gadget: dummy-hcd: Fix shift-out-of-bounds bug Alan Stern

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).