Skip to content

Commit aafb00a

Browse files
jhovoldgregkh
authored andcommitted
USB: yurex: fix NULL-derefs on disconnect
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 ef61eb4 ("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: aadd647 ("USB: yurex.c: remove dbg() usage") Fixes: 4571410 ("USB: yurex.c: remove err() usage") Cc: stable <[email protected]> # 3.5: ef61eb4 Signed-off-by: Johan Hovold <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent ebb2fe5 commit aafb00a

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

drivers/usb/misc/yurex.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ struct usb_yurex {
6060

6161
struct kref kref;
6262
struct mutex io_mutex;
63+
unsigned long disconnected:1;
6364
struct fasync_struct *async_queue;
6465
wait_queue_head_t waitq;
6566

@@ -107,6 +108,7 @@ static void yurex_delete(struct kref *kref)
107108
dev->int_buffer, dev->urb->transfer_dma);
108109
usb_free_urb(dev->urb);
109110
}
111+
usb_put_intf(dev->interface);
110112
usb_put_dev(dev->udev);
111113
kfree(dev);
112114
}
@@ -205,7 +207,7 @@ static int yurex_probe(struct usb_interface *interface, const struct usb_device_
205207
init_waitqueue_head(&dev->waitq);
206208

207209
dev->udev = usb_get_dev(interface_to_usbdev(interface));
208-
dev->interface = interface;
210+
dev->interface = usb_get_intf(interface);
209211

210212
/* set up the endpoint information */
211213
iface_desc = interface->cur_altsetting;
@@ -316,8 +318,9 @@ static void yurex_disconnect(struct usb_interface *interface)
316318

317319
/* prevent more I/O from starting */
318320
usb_poison_urb(dev->urb);
321+
usb_poison_urb(dev->cntl_urb);
319322
mutex_lock(&dev->io_mutex);
320-
dev->interface = NULL;
323+
dev->disconnected = 1;
321324
mutex_unlock(&dev->io_mutex);
322325

323326
/* wakeup waiters */
@@ -405,7 +408,7 @@ static ssize_t yurex_read(struct file *file, char __user *buffer, size_t count,
405408
dev = file->private_data;
406409

407410
mutex_lock(&dev->io_mutex);
408-
if (!dev->interface) { /* already disconnected */
411+
if (dev->disconnected) { /* already disconnected */
409412
mutex_unlock(&dev->io_mutex);
410413
return -ENODEV;
411414
}
@@ -440,7 +443,7 @@ static ssize_t yurex_write(struct file *file, const char __user *user_buffer,
440443
goto error;
441444

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

0 commit comments

Comments
 (0)