WARNING in usb_submit_urb (3)

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@rowland.harvard.edu> 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@syzkaller.appspotmail.com
> >>
> >> 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",

WARNING in usb_submit_urb (3)

[Andrey Konovalov]

On Mon, Oct 15, 2018 at 7:12 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> 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?

You can ask syzbot to do this:

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

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 244417d0dfd1..ffccd40ea67d 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1474,8 +1474,6 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 	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_dev_state *ps, struct usbdevfs_urb *uurb
 			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",

WARNING in usb_submit_urb (3)

[syzbot]

Hello,

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

Reported-and-tested-by:  
syzbot+24a30223a4b609bb802e@syzkaller.appspotmail.com

Tested on:

commit:         f0a7d1883d9f afs: Fix clearance of reply
git tree:       upstream
kernel config:  https://syzkaller.appspot.com/x/.config?x=b3f55cb3dfcc6c33
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=164eab91400000

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

WARNING in usb_submit_urb (3)

[Oliver Neukum]

On Mo, 2018-10-15 at 13:12 -0400, Alan Stern wrote:
> On Mon, 15 Oct 2018, Andrey Konovalov wrote:
> 
> 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
> 
Thanks for the catch.

	Regards
		Oliver