Make sure to always cancel the control URB in write() so that it can be reused after a timeout or spurious CMD_ACK.
Currently any further write requests after a timeout would fail after triggering a WARN() in usb_submit_urb() when attempting to submit the already active URB.
Reported-by: syzbot+e87ebe0f7913f71f2ea5@syzkaller.appspotmail.com Fixes: 6bc235a2e24a ("USB: add driver for Meywa-Denki & Kayac YUREX") Cc: stable stable@vger.kernel.org # 2.6.37 Signed-off-by: Johan Hovold johan@kernel.org --- drivers/usb/misc/yurex.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/misc/yurex.c b/drivers/usb/misc/yurex.c index 73ebfa6e9715..c640f98d20c5 100644 --- a/drivers/usb/misc/yurex.c +++ b/drivers/usb/misc/yurex.c @@ -496,6 +496,9 @@ static ssize_t yurex_write(struct file *file, const char __user *user_buffer, timeout = schedule_timeout(YUREX_WRITE_TIMEOUT); finish_wait(&dev->waitq, &wait);
+ /* make sure URB is idle after timeout or (spurious) CMD_ACK */ + usb_kill_urb(dev->cntl_urb); + mutex_unlock(&dev->io_mutex);
if (retval < 0) {
On Mon, Dec 14, 2020 at 11:44:44AM +0100, Johan Hovold wrote:
Make sure to always cancel the control URB in write() so that it can be reused after a timeout or spurious CMD_ACK.
Currently any further write requests after a timeout would fail after triggering a WARN() in usb_submit_urb() when attempting to submit the already active URB.
Reported-by: syzbot+e87ebe0f7913f71f2ea5@syzkaller.appspotmail.com Fixes: 6bc235a2e24a ("USB: add driver for Meywa-Denki & Kayac YUREX") Cc: stable stable@vger.kernel.org # 2.6.37 Signed-off-by: Johan Hovold johan@kernel.org
Forgot linux-usb...
Let's try this too:
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
drivers/usb/misc/yurex.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/misc/yurex.c b/drivers/usb/misc/yurex.c index 73ebfa6e9715..c640f98d20c5 100644 --- a/drivers/usb/misc/yurex.c +++ b/drivers/usb/misc/yurex.c @@ -496,6 +496,9 @@ static ssize_t yurex_write(struct file *file, const char __user *user_buffer, timeout = schedule_timeout(YUREX_WRITE_TIMEOUT); finish_wait(&dev->waitq, &wait);
- /* make sure URB is idle after timeout or (spurious) CMD_ACK */
- usb_kill_urb(dev->cntl_urb);
- mutex_unlock(&dev->io_mutex);
if (retval < 0) {
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue: WARNING in yurex_write/usb_submit_urb
------------[ cut here ]------------ URB 00000000d1c13d63 submitted while active WARNING: CPU: 1 PID: 12383 at drivers/usb/core/urb.c:378 usb_submit_urb+0x1228/0x14e0 drivers/usb/core/urb.c:378 Modules linked in: CPU: 1 PID: 12383 Comm: syz-executor.2 Not tainted 5.10.0-rc7-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:usb_submit_urb+0x1228/0x14e0 drivers/usb/core/urb.c:378 Code: 89 de e8 1b cd 3b fc 84 db 0f 85 da f4 ff ff e8 fe d4 3b fc 4c 89 fe 48 c7 c7 20 59 e1 89 c6 05 14 8c a4 07 01 e8 a4 b3 78 03 <0f> 0b e9 b8 f4 ff ff c7 44 24 14 01 00 00 00 e9 6f f5 ff ff 41 bd RSP: 0018:ffffc90001c9fcb8 EFLAGS: 00010282 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: ffff888020788000 RSI: ffffffff8158c835 RDI: fffff52000393f89 RBP: 1ffff92000393fa9 R08: 0000000000000001 R09: ffff8880b9f30627 R10: 0000000000000000 R11: 0000000000000000 R12: ffff8880276ea400 R13: 00000000fffffff0 R14: ffff8880276ea4e8 R15: ffff888012dca100 FS: 00007fad9e6f7700(0000) GS:ffff8880b9f00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00005587bf0f4160 CR3: 00000000252a1000 CR4: 00000000001506e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: yurex_write+0x3f4/0x840 drivers/usb/misc/yurex.c:494 vfs_write+0x28e/0xa30 fs/read_write.c:603 ksys_write+0x12d/0x250 fs/read_write.c:658 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x45e159 Code: 0d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 db b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007fad9e6f6c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000045e159 RDX: 0000000000000001 RSI: 0000000020000740 RDI: 0000000000000004 RBP: 000000000119bfc0 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 000000000119bf8c R13: 00007ffc9fc101ef R14: 00007fad9e6f79c0 R15: 000000000119bf8c
Tested on:
commit: a256e240 usb: phy: convert comma to semicolon git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing console output: https://syzkaller.appspot.com/x/log.txt?x=17357137500000 kernel config: https://syzkaller.appspot.com/x/.config?x=4536e7f93c2bc8e9 dashboard link: https://syzkaller.appspot.com/bug?extid=e87ebe0f7913f71f2ea5 compiler: gcc (GCC) 10.1.0-syz 20200507
On Mon, Dec 14, 2020 at 06:48:03AM -0800, syzbot wrote:
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue: WARNING in yurex_write/usb_submit_urb
It appears syzbot never tested the patch from the thread. Probably using it's mail interface incorrectly, I don't know and I don't have time to investigate. The patch itself is correct.
Johan
On Mon, Dec 14, 2020 at 4:02 PM Johan Hovold johan@kernel.org wrote:
On Mon, Dec 14, 2020 at 06:48:03AM -0800, syzbot wrote:
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue: WARNING in yurex_write/usb_submit_urb
It appears syzbot never tested the patch from the thread. Probably using it's mail interface incorrectly, I don't know and I don't have time to investigate. The patch itself is correct.
Hi Johan,
I wasn't CCed on the testing request, so I can't say what exactly was wrong.
Could you send me the patch you were trying to test?
Thanks!
On Mon, Dec 14, 2020 at 04:06:49PM +0100, Andrey Konovalov wrote:
On Mon, Dec 14, 2020 at 4:02 PM Johan Hovold johan@kernel.org wrote:
On Mon, Dec 14, 2020 at 06:48:03AM -0800, syzbot wrote:
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue: WARNING in yurex_write/usb_submit_urb
It appears syzbot never tested the patch from the thread. Probably using it's mail interface incorrectly, I don't know and I don't have time to investigate. The patch itself is correct.
Hi Johan,
I wasn't CCed on the testing request, so I can't say what exactly was wrong.
Here's the patch and the "syz test" command in a reply:
https://lore.kernel.org/r/20201214104444.28386-1-johan@kernel.org
Probably needs to go in the same mail, right?
How about including the command needed to test a patch in the syzbot report mail to assist the casual user of its interfaces? I had to browse the web page you link to and still got it wrong apparently.
Could you send me the patch you were trying to test?
Does this work better:
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
Johan
From 3bb77b2ac604d70b06f45a850e326dda9c99c9cd Mon Sep 17 00:00:00 2001
From: Johan Hovold johan@kernel.org Date: Mon, 14 Dec 2020 11:30:53 +0100 Subject: [PATCH] USB: yurex: fix control-URB timeout handling
Make sure to always cancel the control URB in write() so that it can be reused after a timeout or spurious CMD_ACK.
Currently any further write requests after a timeout would fail after triggering a WARN() in usb_submit_urb() when attempting to submit the already active URB.
Reported-by: syzbot+e87ebe0f7913f71f2ea5@syzkaller.appspotmail.com Fixes: 6bc235a2e24a ("USB: add driver for Meywa-Denki & Kayac YUREX") Cc: stable stable@vger.kernel.org # 2.6.37 Signed-off-by: Johan Hovold johan@kernel.org --- drivers/usb/misc/yurex.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/misc/yurex.c b/drivers/usb/misc/yurex.c index 73ebfa6e9715..c640f98d20c5 100644 --- a/drivers/usb/misc/yurex.c +++ b/drivers/usb/misc/yurex.c @@ -496,6 +496,9 @@ static ssize_t yurex_write(struct file *file, const char __user *user_buffer, timeout = schedule_timeout(YUREX_WRITE_TIMEOUT); finish_wait(&dev->waitq, &wait);
+ /* make sure URB is idle after timeout or (spurious) CMD_ACK */ + usb_kill_urb(dev->cntl_urb); + mutex_unlock(&dev->io_mutex);
if (retval < 0) {
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: syzbot+e87ebe0f7913f71f2ea5@syzkaller.appspotmail.com
Tested on:
commit: a256e240 usb: phy: convert comma to semicolon git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing kernel config: https://syzkaller.appspot.com/x/.config?x=4536e7f93c2bc8e9 dashboard link: https://syzkaller.appspot.com/bug?extid=e87ebe0f7913f71f2ea5 compiler: gcc (GCC) 10.1.0-syz 20200507 patch: https://syzkaller.appspot.com/x/patch.diff?x=11a9f703500000
Note: testing is done by a robot and is best-effort only.
On Mon, Dec 14, 2020 at 4:16 PM Johan Hovold johan@kernel.org wrote:
On Mon, Dec 14, 2020 at 04:06:49PM +0100, Andrey Konovalov wrote:
On Mon, Dec 14, 2020 at 4:02 PM Johan Hovold johan@kernel.org wrote:
On Mon, Dec 14, 2020 at 06:48:03AM -0800, syzbot wrote:
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue: WARNING in yurex_write/usb_submit_urb
It appears syzbot never tested the patch from the thread. Probably using it's mail interface incorrectly, I don't know and I don't have time to investigate. The patch itself is correct.
Hi Johan,
I wasn't CCed on the testing request, so I can't say what exactly was wrong.
Here's the patch and the "syz test" command in a reply:
https://lore.kernel.org/r/20201214104444.28386-1-johan@kernel.orgProbably needs to go in the same mail, right?
Yes, both the syz test command and the patch must be in the same email, which is sent as a response to the initial report.
How about including the command needed to test a patch in the syzbot report mail to assist the casual user of its interfaces? I had to browse the web page you link to and still got it wrong apparently.
I think it's deliberately not included to not overload the report email with too many details.
Could you send me the patch you were trying to test?
Does this work better:
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
This worked :)
Thanks!
linux-stable-mirror@lists.linaro.org
-
Andrey Konovalov -
Johan Hovold -
syzbot