Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 28f2c946 authored by Johan Hovold's avatar Johan Hovold Committed by Greg Kroah-Hartman
Browse files

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: 9d974b2a ("USB: legousbtower.c: remove err() usage")
Fixes: fef526ca ("USB: legousbtower: remove custom debug macro")
Fixes: 4dae9963 ("USB: legotower: remove custom debug macro and module parameter")
Cc: stable <stable@vger.kernel.org>     # 3.5
Signed-off-by: default avatarJohan Hovold <johan@kernel.org>
Link: https://lore.kernel.org/r/20190919083039.30898-4-johan@kernel.org


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 2e0bcd20

drivers/usb/misc/legousbtower.c

+15 −11
Original line number Diff line number Diff line
@@ -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_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);
@@ -436,7 +435,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 */
@@ -472,10 +472,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);

	}

	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 file *file, poll_table *wait) 


	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 *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;
@@ -644,7 +643,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;
@@ -753,7 +752,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,
@@ -823,6 +822,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;
@@ -970,6 +970,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 */
@@ -977,7 +981,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);