Re: WARNING in usb_submit_urb (3)

[Alan Stern]

On Mon, 15 Oct 2018, Andrey Konovalov wrote:

> On Mon, Oct 15, 2018 at 5:22 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Fri, 12 Oct 2018, syzbot wrote:
> >
> >> Hello,
> >>
> >> syzbot found the following crash on:
> >>
> >> HEAD commit:    9dcd936c5312 Merge tag 'for-4.19/dm-fixes-4' of git://git...
> >> git tree:       upstream
> >> console output: https://syzkaller.appspot.com/x/log.txt?x=123b8da1400000
> >> kernel config:  https://syzkaller.appspot.com/x/.config?x=88e9a8a39dc0be2d
> >> dashboard link: https://syzkaller.appspot.com/bug?extid=24a30223a4b609bb802e
> >> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13888991400000
> >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1476e5e6400000
> >>
> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> Reported-by: syzbot+24a30223a4b609bb802e@xxxxxxxxxxxxxxxxxxxxxxxxx
> >>
> >> IPVS: ftp: loaded support on port[0] = 21
> >> ------------[ cut here ]------------
> >> usb usb7: BOGUS urb flags, 1 --> 0
> >> WARNING: CPU: 0 PID: 5828 at drivers/usb/core/urb.c:503
> >> usb_submit_urb+0x717/0x14e0 drivers/usb/core/urb.c:502
> >> Kernel panic - not syncing: panic_on_warn set ...
> >
> > This should have been fixed by commit 7a68d9fb8510 ("USB: usbdevfs:
> > sanitize flags more").  Was that commit not present in the kernel you
> > tested?
> 
> The commit is there, AFAICT. This must be a different issue.

Ah, I see the problem.  In fact it is the same issue, but the commit
mentioned above contains an error (is_in gets tested too soon).  The
fix is below; can you check it?

Alan Stern



Index: usb-4.x/drivers/usb/core/devio.c
===================================================================
--- usb-4.x.orig/drivers/usb/core/devio.c
+++ usb-4.x/drivers/usb/core/devio.c
@@ -1474,8 +1474,6 @@ static int proc_do_submiturb(struct usb_
 	u = 0;
 	switch (uurb->type) {
 	case USBDEVFS_URB_TYPE_CONTROL:
-		if (is_in)
-			allow_short = true;
 		if (!usb_endpoint_xfer_control(&ep->desc))
 			return -EINVAL;
 		/* min 8 byte setup packet */
@@ -1505,6 +1503,8 @@ static int proc_do_submiturb(struct usb_
 			is_in = 0;
 			uurb->endpoint &= ~USB_DIR_IN;
 		}
+		if (is_in)
+			allow_short = true;
 		snoop(&ps->dev->dev, "control urb: bRequestType=%02x "
 			"bRequest=%02x wValue=%04x "
 			"wIndex=%04x wLength=%04x\n",