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

[Oliver Neukum]

Am Donnerstag, den 23.04.2020, 12:29 -0400 schrieb Alan Stern:
> On Thu, 23 Apr 2020, Oliver Neukum wrote:

> The only suspicious thing I see is that usblp_resume() calls 
> handle_bidir() without first acquiring any mutex.  But resume shouldn't 
> race with disconnect.

Right.

> The only other place where read URBs get submitted is under
> usblp_read(), which does acquire the mutex

Right.

>  and checks for disconnection
> while holding it.

Where? It should, but I do not see where it does so.

	Regards
		Oliver
From 89db5232b4df56972d284c12fd1bb8e44fb81e7d Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@xxxxxxxx>
Date: Wed, 22 Apr 2020 13:14:25 +0200
Subject: [PATCH] usblp: fix race between disconnect() and read()

read() needs to check whether the device has been
disconnected before it tries to talk to the device.

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

diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 0d8e3f3804a3..fbc8298c5f84 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -826,6 +826,11 @@ static ssize_t usblp_read(struct file *file, char __user *buffer, size_t len, lo
 	if (rv < 0)
 		return rv;
 
+	if (!usblp->present) {
+		count = -ENODEV;
+		goto done;
+	}
+
 	if ((avail = usblp->rstatus) < 0) {
 		printk(KERN_ERR "usblp%d: error %d reading from printer\n",
 		    usblp->minor, (int)avail);
-- 
2.16.4