lkml.org 
[lkml]   [2019]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH 4.9 70/92] USB: legousbtower: fix potential NULL-deref on disconnect
Date
From: Johan Hovold <johan@kernel.org>

commit cd81e6fa8e033e7bcd59415b4a65672b4780030b upstream.

The driver is using its struct usb_device pointer as an inverted
disconnected flag, but was setting it to NULL before making sure all
completion handlers had run. This could lead to a NULL-pointer
dereference in a number of dev_dbg and dev_err statements in the
completion handlers which relies on said pointer.

Fix this by unconditionally stopping all I/O and preventing
resubmissions by poisoning the interrupt URBs at disconnect and using a
dedicated disconnected flag.

This also makes sure that all I/O has completed by the time the
disconnect callback returns.

Fixes: 9d974b2a06e3 ("USB: legousbtower.c: remove err() usage")
Fixes: fef526cae700 ("USB: legousbtower: remove custom debug macro")
Fixes: 4dae99638097 ("USB: legotower: remove custom debug macro and module parameter")
Cc: stable <stable@vger.kernel.org> # 3.5
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20190919083039.30898-4-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
drivers/usb/misc/legousbtower.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)

--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -196,6 +196,7 @@ struct lego_usb_tower {
unsigned char minor; /* the starting minor number for this device */

int open_count; /* number of times this port has been opened */
+ unsigned long disconnected:1;

char* read_buffer;
size_t read_buffer_length; /* this much came in */
@@ -295,8 +296,6 @@ static inline void lego_usb_tower_debug_
*/
static inline void tower_delete (struct lego_usb_tower *dev)
{
- tower_abort_transfers (dev);
-
/* free data structures */
usb_free_urb(dev->interrupt_in_urb);
usb_free_urb(dev->interrupt_out_urb);
@@ -436,7 +435,8 @@ static int tower_release (struct inode *
retval = -ENODEV;
goto unlock_exit;
}
- if (dev->udev == NULL) {
+
+ if (dev->disconnected) {
/* the device was unplugged before the file was released */

/* unlock here as tower_delete frees dev */
@@ -472,10 +472,9 @@ static void tower_abort_transfers (struc
if (dev->interrupt_in_running) {
dev->interrupt_in_running = 0;
mb();
- if (dev->udev)
- usb_kill_urb (dev->interrupt_in_urb);
+ usb_kill_urb(dev->interrupt_in_urb);
}
- if (dev->interrupt_out_busy && dev->udev)
+ if (dev->interrupt_out_busy)
usb_kill_urb(dev->interrupt_out_urb);
}

@@ -511,7 +510,7 @@ static unsigned int tower_poll (struct f

dev = file->private_data;

- if (!dev->udev)
+ if (dev->disconnected)
return POLLERR | POLLHUP;

poll_wait(file, &dev->read_wait, wait);
@@ -558,7 +557,7 @@ static ssize_t tower_read (struct file *
}

/* verify that the device wasn't unplugged */
- if (dev->udev == NULL) {
+ if (dev->disconnected) {
retval = -ENODEV;
pr_err("No device or device unplugged %d\n", retval);
goto unlock_exit;
@@ -644,7 +643,7 @@ static ssize_t tower_write (struct file
}

/* verify that the device wasn't unplugged */
- if (dev->udev == NULL) {
+ if (dev->disconnected) {
retval = -ENODEV;
pr_err("No device or device unplugged %d\n", retval);
goto unlock_exit;
@@ -753,7 +752,7 @@ static void tower_interrupt_in_callback

resubmit:
/* resubmit if we're still running */
- if (dev->interrupt_in_running && dev->udev) {
+ if (dev->interrupt_in_running) {
retval = usb_submit_urb (dev->interrupt_in_urb, GFP_ATOMIC);
if (retval)
dev_err(&dev->udev->dev,
@@ -821,6 +820,7 @@ static int tower_probe (struct usb_inter

dev->udev = udev;
dev->open_count = 0;
+ dev->disconnected = 0;

dev->read_buffer = NULL;
dev->read_buffer_length = 0;
@@ -958,6 +958,10 @@ static void tower_disconnect (struct usb
/* give back our minor and prevent further open() */
usb_deregister_dev (interface, &tower_class);

+ /* stop I/O */
+ usb_poison_urb(dev->interrupt_in_urb);
+ usb_poison_urb(dev->interrupt_out_urb);
+
mutex_lock(&dev->lock);

/* if the device is not opened, then we clean up right now */
@@ -965,7 +969,7 @@ static void tower_disconnect (struct usb
mutex_unlock(&dev->lock);
tower_delete (dev);
} else {
- dev->udev = NULL;
+ dev->disconnected = 1;
/* wake up pollers */
wake_up_interruptible_all(&dev->read_wait);
wake_up_interruptible_all(&dev->write_wait);

\
 
 \ /
  Last update: 2019-10-17 00:17    [W:0.296 / U:2.672 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site