]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
USB: iowarrior: fix use-after-free on disconnect race
authorJohan Hovold <johan@kernel.org>
Mon, 22 Jun 2026 15:26:09 +0000 (17:26 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 25 Jun 2026 14:14:48 +0000 (15:14 +0100)
mutex_unlock() may access the mutex structure after releasing the lock
and therefore cannot be used to manage lifetime of objects directly
(unlike spinlocks and refcounts). [1][2]

Use a kref to release the driver data to avoid use-after-free in
mutex_unlock() when release() races with disconnect().

[1] a51749ab34d9 ("locking/mutex: Document that mutex_unlock() is non-atomic")
[2] 2b9d9e0a9ba0 ("locking/mutex: Clarify that mutex_unlock(), and most
                   other sleeping locks, can still use the lock object
                   after it's unlocked")

Fixes: 946b960d13c1 ("USB: add driver for iowarrior devices.")
Cc: stable <stable@kernel.org>
Reported-by: Yue Sun <samsun1006219@gmail.com>
Link: https://lore.kernel.org/r/20260618080204.38322-1-samsun1006219@gmail.com
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://patch.msgid.link/20260622152612.116422-2-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/misc/iowarrior.c

index 88c6d1d1da11625474a65a03aba1e977f176bcb0..de2b236ef9030d8be15d4154b55a9f7dab7c0f7f 100644 (file)
@@ -72,6 +72,7 @@ static struct usb_driver iowarrior_driver;
 
 /* Structure to hold all of our device specific stuff */
 struct iowarrior {
+       struct kref kref;
        struct mutex mutex;                     /* locks this structure */
        struct usb_device *udev;                /* save off the usb device pointer */
        struct usb_interface *interface;        /* the interface for this device */
@@ -240,8 +241,10 @@ static void iowarrior_write_callback(struct urb *urb)
 /*
  *     iowarrior_delete
  */
-static inline void iowarrior_delete(struct iowarrior *dev)
+static inline void iowarrior_delete(struct kref *kref)
 {
+       struct iowarrior *dev = container_of(kref, struct iowarrior, kref);
+
        kfree(dev->int_in_buffer);
        usb_free_urb(dev->int_in_urb);
        kfree(dev->read_queue);
@@ -637,6 +640,9 @@ static int iowarrior_open(struct inode *inode, struct file *file)
        }
        /* increment our usage count for the driver */
        ++dev->opened;
+
+       kref_get(&dev->kref);
+
        /* save our object in the file's private structure */
        file->private_data = dev;
        retval = 0;
@@ -652,7 +658,6 @@ out:
 static int iowarrior_release(struct inode *inode, struct file *file)
 {
        struct iowarrior *dev;
-       int retval = 0;
 
        dev = file->private_data;
        if (!dev)
@@ -660,29 +665,18 @@ static int iowarrior_release(struct inode *inode, struct file *file)
 
        /* lock our device */
        mutex_lock(&dev->mutex);
+       dev->opened = 0;        /* we're closing now */
 
-       if (dev->opened <= 0) {
-               retval = -ENODEV;       /* close called more than once */
-               mutex_unlock(&dev->mutex);
-       } else {
-               dev->opened = 0;        /* we're closing now */
-               retval = 0;
-               if (dev->present) {
-                       /*
-                          The device is still connected so we only shutdown
-                          pending read-/write-ops.
-                        */
-                       usb_kill_urb(dev->int_in_urb);
-                       wake_up_interruptible(&dev->read_wait);
-                       wake_up_interruptible(&dev->write_wait);
-                       mutex_unlock(&dev->mutex);
-               } else {
-                       /* The device was unplugged, cleanup resources */
-                       mutex_unlock(&dev->mutex);
-                       iowarrior_delete(dev);
-               }
+       if (dev->present) {
+               usb_kill_urb(dev->int_in_urb);
+               wake_up_interruptible(&dev->read_wait);
+               wake_up_interruptible(&dev->write_wait);
        }
-       return retval;
+       mutex_unlock(&dev->mutex);
+
+       kref_put(&dev->kref, iowarrior_delete);
+
+       return 0;
 }
 
 static __poll_t iowarrior_poll(struct file *file, poll_table * wait)
@@ -767,6 +761,7 @@ static int iowarrior_probe(struct usb_interface *interface,
        if (!dev)
                return retval;
 
+       kref_init(&dev->kref);
        mutex_init(&dev->mutex);
 
        atomic_set(&dev->intr_idx, 0);
@@ -885,7 +880,8 @@ static int iowarrior_probe(struct usb_interface *interface,
        return retval;
 
 error:
-       iowarrior_delete(dev);
+       kref_put(&dev->kref, iowarrior_delete);
+
        return retval;
 }
 
@@ -909,19 +905,14 @@ static void iowarrior_disconnect(struct usb_interface *interface)
        usb_kill_anchored_urbs(&dev->submitted);
 
        if (dev->opened) {
-               /* There is a process that holds a filedescriptor to the device ,
-                  so we only shutdown read-/write-ops going on.
-                  Deleting the device is postponed until close() was called.
-                */
                usb_kill_urb(dev->int_in_urb);
                wake_up_interruptible(&dev->read_wait);
                wake_up_interruptible(&dev->write_wait);
-               mutex_unlock(&dev->mutex);
-       } else {
-               /* no process is using the device, cleanup now */
-               mutex_unlock(&dev->mutex);
-               iowarrior_delete(dev);
        }
+
+       mutex_unlock(&dev->mutex);
+
+       kref_put(&dev->kref, iowarrior_delete);
 }
 
 /* usb specific object needed to register this driver with the usb subsystem */