[PATCH 3.16 058/136] USB: yurex: fix NULL-derefs on disconnect

From: Ben Hutchings
Date: Mon Dec 16 2019 - 19:58:56 EST


3.16.80-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Johan Hovold <johan@xxxxxxxxxx>

commit aafb00a977cf7d81821f7c9d12e04c558c22dc3c upstream.

The driver was using its struct usb_interface pointer as an inverted
disconnected flag, but was setting it to NULL without making sure all
code paths that used it were done with it.

Before commit ef61eb43ada6 ("USB: yurex: Fix protection fault after
device removal") this included the interrupt-in completion handler, but
there are further accesses in dev_err and dev_dbg statements in
yurex_write() and the driver-data destructor (sic!).

Fix this by unconditionally stopping also the control URB at disconnect
and by using a dedicated disconnected flag.

Note that we need to take a reference to the struct usb_interface to
avoid a use-after-free in the destructor whenever the device was
disconnected while the character device was still open.

Fixes: aadd6472d904 ("USB: yurex.c: remove dbg() usage")
Fixes: 45714104b9e8 ("USB: yurex.c: remove err() usage")
Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20191009153848.8664-6-johan@xxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
drivers/usb/misc/yurex.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

--- a/drivers/usb/misc/yurex.c
+++ b/drivers/usb/misc/yurex.c
@@ -64,6 +64,7 @@ struct usb_yurex {

struct kref kref;
struct mutex io_mutex;
+ unsigned long disconnected:1;
struct fasync_struct *async_queue;
wait_queue_head_t waitq;

@@ -111,6 +112,7 @@ static void yurex_delete(struct kref *kr
dev->int_buffer, dev->urb->transfer_dma);
usb_free_urb(dev->urb);
}
+ usb_put_intf(dev->interface);
usb_put_dev(dev->udev);
kfree(dev);
}
@@ -211,7 +213,7 @@ static int yurex_probe(struct usb_interf
init_waitqueue_head(&dev->waitq);

dev->udev = usb_get_dev(interface_to_usbdev(interface));
- dev->interface = interface;
+ dev->interface = usb_get_intf(interface);

/* set up the endpoint information */
iface_desc = interface->cur_altsetting;
@@ -334,8 +336,9 @@ static void yurex_disconnect(struct usb_

/* prevent more I/O from starting */
usb_poison_urb(dev->urb);
+ usb_poison_urb(dev->cntl_urb);
mutex_lock(&dev->io_mutex);
- dev->interface = NULL;
+ dev->disconnected = 1;
mutex_unlock(&dev->io_mutex);

/* wakeup waiters */
@@ -422,7 +425,7 @@ static ssize_t yurex_read(struct file *f
dev = (struct usb_yurex *)file->private_data;

mutex_lock(&dev->io_mutex);
- if (!dev->interface) { /* already disconnected */
+ if (dev->disconnected) { /* already disconnected */
mutex_unlock(&dev->io_mutex);
return -ENODEV;
}
@@ -453,7 +456,7 @@ static ssize_t yurex_write(struct file *
goto error;

mutex_lock(&dev->io_mutex);
- if (!dev->interface) { /* already disconnected */
+ if (dev->disconnected) { /* already disconnected */
mutex_unlock(&dev->io_mutex);
retval = -ENODEV;
goto error;