Skip to contentSkip to conversation

INFO: task hung in wdm_flush

DISCUSSION Oliver Neukum 3 participants 4 messages Nov 19, 2019 – Nov 22, 2019 USB

lore
Share & tools
lore
#0 lore raw

#syz test: https://github.com/google/kasan.git e96407b4

From d3d9edf17e33889e0fc4238f3d03a2dce7af30e1 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <[email protected]>
Date: Tue, 19 Nov 2019 14:09:41 +0100
Subject: [PATCH] cdc-wdm: add timeout in wdm_flush()

wdm_flush() will wait forever for IO to end. If a device happens to crash exactly at that time and becomes unresponsive or turns rogue and malicious exactly at that time, we get unkillable tasks. The solition is to add a sensible timeout.

---
 drivers/usb/class/cdc-wdm.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/usb/class/cdc-wdm.c +10 -2
index f9f7c8a5e091..17de5c88a325 100644
@@ -587,8 +587,9 @@ static ssize_t wdm_read
 static int wdm_flush(struct file *file, fl_owner_t id)
 {
 	struct wdm_device *desc = file->private_data;
+	int timeout;
 
-	wait_event(desc->wait,
+	timeout = wait_event_timeout(desc->wait,
 			/*
 			 * needs both flags. We cannot do with one
 			 * because resetting it would cause a race
@@ -596,7 +597,14 @@ static int wdm_flush(struct file *file, fl_owner_t id)
 			 * a disconnect
 			 */
 			!test_bit(WDM_IN_USE, &desc->flags) ||
-			test_bit(WDM_DISCONNECTING, &desc->flags));
+			test_bit(WDM_DISCONNECTING, &desc->flags),
+			/* pulled out of thin air */
+			30 * HZ);
+
+	if (!timeout) {
+		usb_kill_urb(desc->command);
+		return -EIO;
+	}
 
 	/* cannot dereference desc->intf if WDM_DISCONNECTING */
 	if (test_bit(WDM_DISCONNECTING, &desc->flags))
Signature · 1 line
2.16.4
syzbot
#1 lore raw
syzbot tried to test the proposed patch but build/boot failed:

Hello,

syzbot tried to test the proposed patch but build/boot failed:

failed to apply patch:
checking file drivers/usb/class/cdc-wdm.c
Hunk #1 FAILED at 587.
Hunk #2 FAILED at 596.
2 out of 2 hunks FAILED

Tested on:

commit:         e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git
dashboard link: https://syzkaller.appspot.com/bug?extid=854768b99f19e89d7f81
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1779956ae00000
#2 lore raw

Am Mittwoch, den 20.11.2019, 14:40 -0800 schrieb syzbot:

Quoted 9 lines

Hello,

syzbot tried to test the proposed patch but build/boot failed:

failed to apply patch:
checking file drivers/usb/class/cdc-wdm.c
Hunk #1 FAILED at 587.
Hunk #2 FAILED at 596.
2 out of 2 hunks FAILED

This is unexpected.

Quoted 7 lines

Tested on:

commit:         e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git

If I do a git am on the branch usb-fuzzer-usb-testing-2019.11.19, the patch applies. Which branch do I need to backport to?

Reagrds
Oliver

#3 lore raw
Quoted 16 lines · Oliver Neukum

Am Mittwoch, den 20.11.2019, 14:40 -0800 schrieb syzbot:

Quoted 9 lines

Hello,

syzbot tried to test the proposed patch but build/boot failed:

failed to apply patch:
checking file drivers/usb/class/cdc-wdm.c
Hunk #1 FAILED at 587.
Hunk #2 FAILED at 596.
2 out of 2 hunks FAILED

This is unexpected.

Quoted 3 lines
Quoted 7 lines
Quoted 4 lines

Tested on:

commit:         e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git

If I do a git am on the branch usb-fuzzer-usb-testing-2019.11.19, the patch applies. Which branch do I need to backport to?

Hi Oliver,

You give exact tree/base commit when you ask syzbot to test. It does not do any second guessing.
Well, if you provide a tree+branch then it's a bit of chasing a moving target b/c HEAD can be updated meanwhile, but you can also give it tree+commit, then that's 100% fixed.