Re: KASAN: use-after-free Read in usblp_bulk_read

From: Oliver Neukum
Date: Wed May 06 2020 - 05:15:16 EST


Am Donnerstag, den 30.04.2020, 11:11 -0400 schrieb Alan Stern:

> KASAN is documented. The difficulty is that this race is obviously
> hard to trigger, and without the ability to reproduce it we can't run
> diagnostics to find the underlying cause.
>
> We can't even ask syzbot to try running tests for us; without a valid
> reproducer it won't agree to rerun the original test program.


Very well. We are not going to find it without exceptional luck. Yet
there may be a real issue, too. We simply do not know. How about the
attached patch?

Regards
Oliver


From 5ed23e0029cf10cf8dbdd790a190d7e2113560ae Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@xxxxxxxx>
Date: Wed, 6 May 2020 11:05:41 +0200
Subject: [PATCH] usblp: poison URBs upon disconnect

syzkaller reported an UB that should have been killed to be active.
We do not understand it, but this should fix the issue if it is real.

Signed-off-by: Oliver Neukum <oneukum@xxxxxxxx>
---
drivers/usb/class/usblp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 0d8e3f3804a3..084c48c5848f 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -468,7 +468,8 @@ static int usblp_release(struct inode *inode, struct file *file)
usb_autopm_put_interface(usblp->intf);

if (!usblp->present) /* finish cleanup from disconnect */
- usblp_cleanup(usblp);
+ usblp_cleanup(usblp); /* any URBs must be dead */
+
mutex_unlock(&usblp_mutex);
return 0;
}
@@ -1375,9 +1376,11 @@ static void usblp_disconnect(struct usb_interface *intf)

usblp_unlink_urbs(usblp);
mutex_unlock(&usblp->mut);
+ usb_poison_anchored_urbs(&usblp->urbs);

if (!usblp->used)
usblp_cleanup(usblp);
+
mutex_unlock(&usblp_mutex);
}

--
2.16.4