WARNING in usb_submit_urb

[syzbot]

Hello,

syzkaller hit the following crash on
36ef71cae353f88fd6e095e2aaa3e5953af1685d
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.
C reproducer is attached
syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers


------------[ cut here ]------------
WARNING: CPU: 3 PID: 2986 at drivers/usb/core/urb.c:498
usb_submit_urb+0xeb9/0x10f0 drivers/usb/core/urb.c:497
Kernel panic - not syncing: panic_on_warn set ...

CPU: 3 PID: 2986 Comm: syzkaller630695 Not tainted
4.14.0-rc5-next-20171018+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
panic+0x1e4/0x41c kernel/panic.c:183
__warn+0x1c4/0x1e0 kernel/panic.c:546
report_bug+0x211/0x2d0 lib/bug.c:183
fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:177
do_trap_no_signal arch/x86/kernel/traps.c:211 [inline]
do_trap+0x260/0x390 arch/x86/kernel/traps.c:260
do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:297
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:310
invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
RIP: 0010:usb_submit_urb+0xeb9/0x10f0 drivers/usb/core/urb.c:497
RSP: 0018:ffff880039ddf3f0 EFLAGS: 00010286
RAX: 0000000000000022 RBX: ffff88006ab64f00 RCX: 0000000000000000
RDX: 0000000000000022 RSI: 1ffff100073bbe3e RDI: ffffed00073bbe72
RBP: ffff880039ddf448 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000002 R14: ffff88006b5d72c0 R15: 000000000000007f
proc_do_submiturb+0x1f53/0x3860 drivers/usb/core/devio.c:1778
proc_submiturb_compat+0x528/0x7e0 drivers/usb/core/devio.c:1999
usbdev_do_ioctl+0x1632/0x3670 drivers/usb/core/devio.c:2475
usbdev_ioctl+0x25/0x30 drivers/usb/core/devio.c:2552
vfs_ioctl fs/ioctl.c:45 [inline]
do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:685
SYSC_ioctl fs/ioctl.c:700 [inline]
SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
entry_SYSCALL_64_fastpath+0x1f/0xbe
RIP: 0033:0x439089
RSP: 002b:00007ffd7d8a88b8 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: ffffffffffffffff RCX: 0000000000439089
RDX: 0000000020274ffa RSI: 00000000802c550a RDI: 0000000000000003
RBP: 0000000000000082 R08: 00000000000000fb R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000000
R13: 0000000000401ce0 R14: 0000000000401d70 R15: 0000000000000000
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.
Please credit me with: Reported-by: syzbot <syzk...@googlegroups.com>

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line.

[Greg KH]

On Tue, Nov 07, 2017 at 08:11:13AM -0800, syzbot wrote:
> Hello,
>
> syzkaller hit the following crash on
> 36ef71cae353f88fd6e095e2aaa3e5953af1685d
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> C reproducer is attached
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers

This is not a crash, you are doing a panic-on-warning, and you send
invalid data to the kernel and it warned about it properly and kept on
working :)

Perhaps maybe not a full WARN_ON() is to be done here?

thanks,

greg k-h

[Alan Stern]

I don't understand how this could have happened. The raw log explains
the problem:

> [ 15.138822] usb usb1: BOGUS urb flags, 2 --> 0
> [ 15.139498] ------------[ cut here ]------------
> [ 15.139955] WARNING: CPU: 3 PID: 2986 at drivers/usb/core/urb.c:498 usb_submit_urb+0xeb9/0x10f0
...
> [ 15.150280] RIP: 0010:usb_submit_urb+0xeb9/0x10f0
...
> [ 15.155166] proc_do_submiturb+0x1f53/0x3860

The "2 --> 0" means that proc_do_submiturb() tried to submit a control
URB (2 = PIPE_CONTROL) to an isochronous endpoint (0 = PIPE_ISOCHRONOUS).
But right near the start of the routine we have:

switch (uurb->type) {
case USBDEVFS_URB_TYPE_CONTROL:
if (!usb_endpoint_xfer_control(&ep->desc))
return -EINVAL;

So how was the warning triggered?

Alan Stern

[Dmitry Vyukov]

I don't know what happened there, but bot provided a repro for this.

[Alan Stern]

On Wed, 8 Nov 2017, Dmitry Vyukov wrote:

> > I don't understand how this could have happened. The raw log explains
> > the problem:
> >
> >> [ 15.138822] usb usb1: BOGUS urb flags, 2 --> 0
> >> [ 15.139498] ------------[ cut here ]------------
> >> [ 15.139955] WARNING: CPU: 3 PID: 2986 at drivers/usb/core/urb.c:498 usb_submit_urb+0xeb9/0x10f0
> > ...
> >> [ 15.150280] RIP: 0010:usb_submit_urb+0xeb9/0x10f0
> > ...
> >> [ 15.155166] proc_do_submiturb+0x1f53/0x3860
> >
> > The "2 --> 0" means that proc_do_submiturb() tried to submit a control
> > URB (2 = PIPE_CONTROL) to an isochronous endpoint (0 = PIPE_ISOCHRONOUS).
> > But right near the start of the routine we have:
> >
> > switch (uurb->type) {
> > case USBDEVFS_URB_TYPE_CONTROL:
> > if (!usb_endpoint_xfer_control(&ep->desc))
> > return -EINVAL;
> >
> > So how was the warning triggered?
>
> I don't know what happened there, but bot provided a repro for this.

Can you provide a reproducer that will run in a 32-bit userspace
environment?

Alan Stern

[Andrey Konovalov]

On Tue, Nov 7, 2017 at 6:58 PM, Alan Stern <st...@rowland.harvard.edu> wrote:

This isn't the "BOGUS urb xfer" warning, this is "BOGUS urb flags". So
2 means the URB_ISO_ASAP flag, which is passed in urb->transfer_flags
but not allowed. And as far as I understand, it gets set because uurb
(which is passed from user space) has USBDEVFS_URB_ISO_ASAP flag set
when passed to proc_do_submiturb().

>
> Alan Stern
>

[Oliver Neukum]

Am Donnerstag, den 09.11.2017, 13:19 +0100 schrieb Andrey Konovalov:
>
> This isn't the "BOGUS urb xfer" warning, this is "BOGUS urb flags". So
> 2 means the URB_ISO_ASAP flag, which is passed in urb->transfer_flags
> but not allowed. And as far as I understand, it gets set because uurb
> (which is passed from user space) has USBDEVFS_URB_ISO_ASAP flag set
> when passed to proc_do_submiturb().

Hi,

yes we should filter better.
Could you test?

Regards
Oliver

[Alan Stern]

> Subject: [PATCH] USB: usbfs: Filter flags passed in from user space
>
> USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
> Improve sanity checking.
>
> Signed-off-by: Oliver Neukum <one...@suse.com>
> ---
> drivers/usb/core/devio.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index c3aaafc25a04..abe6457516a2 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1473,6 +1473,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb

> case USBDEVFS_URB_TYPE_CONTROL:
> if (!usb_endpoint_xfer_control(&ep->desc))
> return -EINVAL;

> + if (uurb->flags & USBDEVFS_URB_ISO_ASAP)
> + return -EINVAL;
> /* min 8 byte setup packet */
> if (uurb->buffer_length < 8)
> return -EINVAL;
> @@ -1511,6 +1513,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
> break;
>
> case USBDEVFS_URB_TYPE_BULK:
> + if (uurb->flags & USBDEVFS_URB_ISO_ASAP)
> + return -EINVAL;
> switch (usb_endpoint_type(&ep->desc)) {
> case USB_ENDPOINT_XFER_CONTROL:
> case USB_ENDPOINT_XFER_ISOC:

You need to check interrupt URBs also. It would be best to have a
single test before the big "switch" statement:

if ((uurb->flags & USBDEVFS_URB_ISO_ASAP) &&
uurb->type != USBDEVFS_URB_TYPE_ISOCHRONOUS)
return -EINVAL;

Alan Stern

[Eric Biggers]

On Tue, Nov 07, 2017 at 08:11:13AM -0800, syzbot wrote:

No longer seeing this. It seems to have been fixed by commit 446f666da9f01, so
telling syzbot so that it can start reporting similar bugs again:

#syz fix: USB: usbfs: Filter flags passed in from user space

- Eric