patch "USB: ldusb: fix NULL-derefs on driver unbind" added to usb-linus
Subsystems (2)
This is a note to let you know that I've just added the patch titled
USB: ldusb: fix NULL-derefs on driver unbind
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From 58ecf131e74620305175a7aa103f81350bb37570 Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Wed, 9 Oct 2019 17:38:46 +0200
Subject: USB: ldusb: fix NULL-derefs on driver unbind
The driver was using its struct usb_interface 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, dev_warn 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.
--- drivers/usb/misc/ldusb.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/usb/misc/ldusb.c +12 -12
@@ -153,6 +153,7 @@ MODULE_PARM_DESC(min_interrupt_out_interval, "Minimum interrupt out interval instruct ld_usb {struct mutex mutex;/* locks this structure */ struct usb_interface *intf;/* save off the usb interface pointer */ +unsigned long disconnected:1 ;int open_count;/* number of times this port has been opened */
@@ -192,12 +193,10 @@ static void ld_usb_abort_transfers(struct ld_usb *dev)/* shutdown transfer */ if (dev->interrupt_in_running) { dev->interrupt_in_running =0 ; -if (dev->intf) -usb_kill_urb (dev->interrupt_in_urb); +usb_kill_urb (dev->interrupt_in_urb); }if (dev->interrupt_out_busy) -if (dev->intf) -usb_kill_urb (dev->interrupt_out_urb); +usb_kill_urb (dev->interrupt_out_urb); }/**
@@ -205,8 +204,6 @@ static void ld_usb_abort_transfers(struct ld_usb *dev)*/ static void ld_usb_delete (struct ld_usb *dev) { -ld_usb_abort_transfers (dev); -/* free data structures */ usb_free_urb (dev->interrupt_in_urb);usb_free_urb (dev->interrupt_out_urb);
@@ -263,7 +260,7 @@ static void ld_usb_interrupt_in_callback(struct urb *urb) resubmit:/* resubmit if we're still running */ -if (dev->interrupt_in_running && !dev->buffer_overflow && dev->intf) { +if (dev->interrupt_in_running && !dev->buffer_overflow) { retval =usb_submit_urb (dev->interrupt_in_urb,GFP_ATOMIC );if (retval) { dev_err(&dev->intf->dev,
@@ -392,7 +389,7 @@ static int ld_usb_release(struct inode *inode, struct file *file) retval = -ENODEV ; goto unlock_exit; } -if (dev->intf ==NULL ) { +if (dev->disconnected) {/* the device was unplugged before the file was released */ mutex_unlock (&dev->mutex);/* unlock here as ld_usb_delete frees dev */
@@ -423,7 +420,7 @@ static __poll_t ld_usb_poll(struct file *file, poll_table *wait) dev = file->private_data; -if (!dev->intf) +if (dev->disconnected)return EPOLLERR |EPOLLHUP ;poll_wait (file, &dev->read_wait, wait);
@@ -462,7 +459,7 @@ static ssize_t ld_usb_read(struct file *file, char __user *buffer, size_t count, }/* verify that the device wasn't unplugged */ -if (dev->intf ==NULL ) { +if (dev->disconnected) { retval = -ENODEV ;printk (KERN_ERR "ldusb: No device or device unplugged %d\n" , retval); goto unlock_exit;
@@ -542,7 +539,7 @@ static ssize_t ld_usb_write(struct file *file, const char __user *buffer, }/* verify that the device wasn't unplugged */ -if (dev->intf ==NULL ) { +if (dev->disconnected) { retval = -ENODEV ;printk (KERN_ERR "ldusb: No device or device unplugged %d\n" , retval); goto unlock_exit;
@@ -764,6 +761,9 @@ static void ld_usb_disconnect(struct usb_interface *intf)/* give back our minor */ usb_deregister_dev (intf, &ld_usb_class); +usb_poison_urb (dev->interrupt_in_urb); +usb_poison_urb (dev->interrupt_out_urb); +mutex_lock (&dev->mutex);/* if the device is not opened, then we clean up right now */
@@ -771,7 +771,7 @@ static void ld_usb_disconnect(struct usb_interface *intf)mutex_unlock (&dev->mutex);ld_usb_delete (dev); }else { - dev->intf =NULL ; + dev->disconnected =1 ;/* wake up pollers */ wake_up_interruptible_all (&dev->read_wait);wake_up_interruptible_all (&dev->write_wait);
Signature · 1 line
2.23.0