| From 7310cdca2d38b68f3a7a98b989cad44b8a3e8f4f Mon Sep 17 00:00:00 2001 |
| From: Johan Hovold <johan@kernel.org> |
| Date: Thu, 19 Sep 2019 10:30:38 +0200 |
| Subject: [PATCH] USB: legousbtower: fix potential NULL-deref on disconnect |
| |
| 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> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c |
| index 773e4188f336..4fa999882635 100644 |
| --- a/drivers/usb/misc/legousbtower.c |
| +++ b/drivers/usb/misc/legousbtower.c |
| @@ -190,6 +190,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 */ |
| @@ -289,8 +290,6 @@ static inline void lego_usb_tower_debug_data(struct device *dev, |
| */ |
| 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); |
| @@ -430,7 +429,8 @@ static int tower_release (struct inode *inode, struct file *file) |
| 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 */ |
| @@ -466,10 +466,9 @@ static void tower_abort_transfers (struct lego_usb_tower *dev) |
| 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); |
| } |
| |
| @@ -505,7 +504,7 @@ static __poll_t tower_poll (struct file *file, poll_table *wait) |
| |
| dev = file->private_data; |
| |
| - if (!dev->udev) |
| + if (dev->disconnected) |
| return EPOLLERR | EPOLLHUP; |
| |
| poll_wait(file, &dev->read_wait, wait); |
| @@ -552,7 +551,7 @@ static ssize_t tower_read (struct file *file, char __user *buffer, size_t count, |
| } |
| |
| /* 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; |
| @@ -638,7 +637,7 @@ static ssize_t tower_write (struct file *file, const char __user *buffer, size_t |
| } |
| |
| /* 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; |
| @@ -748,7 +747,7 @@ static void tower_interrupt_in_callback (struct urb *urb) |
| |
| 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, |
| @@ -813,6 +812,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device |
| |
| dev->udev = udev; |
| dev->open_count = 0; |
| + dev->disconnected = 0; |
| |
| dev->read_buffer = NULL; |
| dev->read_buffer_length = 0; |
| @@ -938,6 +938,10 @@ static void tower_disconnect (struct usb_interface *interface) |
| /* 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 */ |
| @@ -945,7 +949,7 @@ static void tower_disconnect (struct usb_interface *interface) |
| 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); |
| -- |
| 2.7.4 |
| |