Serialize usb-storage operations with usbfs and 'cat /proc/bus/usb/devices',
so that they cannot disturb storage by seemingly harmless control reads.
This patch was adapted from 2.4 patches by Pete Zaitcev. The initial patch of
the series dates back to 2004 and it unfortunately wasn't applied to 2.6 in the
same form back then (it was applied in another form and immediately reverted).
Despite 8+ years passing from that moment, the vendors didn't stop producing
USB devices that require this kind of patch. Two recent examples are SanDisk
Cruzer Slice 8GB and Kingston DataTraveller 100 G2 32GB. In the latter case,
even the enumeration fails as the INQUIRY command normally takes 2.8 seconds to
finish, so 'udev' also comes into action with its control requests, with neither
completing normally.
Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
This patch is atop of 'usb-linus' branch of Greg's tree.
It has been boot tested with non-buggy USB stick only.
drivers/usb/core/devices.c | 13 +++++++-
drivers/usb/core/devio.c | 26 +++++++++++++++--
drivers/usb/core/usb.c | 61 ++++++++++++++++++++++++++++++++++++++++
drivers/usb/storage/transport.c | 11 +++++++
include/linux/usb.h | 19 ++++++++++++
5 files changed, 126 insertions(+), 4 deletions(-)
Index: usb/drivers/usb/core/devices.c
===================================================================
--- usb.orig/drivers/usb/core/devices.c
+++ usb/drivers/usb/core/devices.c
@@ -423,21 +423,30 @@ static char *usb_dump_desc(char *start,
if (start > end)
return start;
+ /*
+ * Grab device's exclusive_access mutex to prevent its driver or
+ * usbfs from using this device while we are accessing it.
+ */
+ usb_excl_lock(dev, 3, 0);
+
start = usb_dump_device_descriptor(start, end, &dev->descriptor);
if (start > end)
- return start;
+ goto out;
start = usb_dump_device_strings(start, end, dev);
for (i = 0; i < dev->descriptor.bNumConfigurations; i++) {
if (start > end)
- return start;
+ goto out;
start = usb_dump_config(dev->speed,
start, end, dev->config + i,
/* active ? */
(dev->config + i) == dev->actconfig);
}
+
+out:
+ usb_excl_unlock(dev, 3);
return start;
}
Index: usb/drivers/usb/core/devio.c
===================================================================
--- usb.orig/drivers/usb/core/devio.c
+++ usb/drivers/usb/core/devio.c
@@ -992,6 +992,10 @@ static int proc_bulk(struct dev_state *p
ret = -EINVAL;
goto done;
}
+ if (usb_excl_lock(dev, 1, 1)) {
+ ret = -ERESTARTSYS;
+ goto done;
+ }
snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, NULL, 0);
usb_unlock_device(dev);
@@ -999,6 +1003,7 @@ static int proc_bulk(struct dev_state *p
usb_lock_device(dev);
snoop_urb(dev, NULL, pipe, len2, i, COMPLETE, tbuf, len2);
+ usb_excl_unlock(dev, 1);
if (!i && len2) {
if (copy_to_user(bulk.data, tbuf, len2)) {
ret = -EFAULT;
@@ -1012,12 +1017,18 @@ static int proc_bulk(struct dev_state *p
goto done;
}
}
+ if (usb_excl_lock(dev, 2, 1)) {
+ ret = -ERESTARTSYS;
+ goto done;
+ }
snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, tbuf, len1);
usb_unlock_device(dev);
i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
usb_lock_device(dev);
snoop_urb(dev, NULL, pipe, len2, i, COMPLETE, NULL, 0);
+
+ usb_excl_unlock(dev, 2);
}
ret = (i < 0 ? i : len2);
done:
@@ -1979,8 +1990,17 @@ static long usbdev_do_ioctl(struct file
usb_lock_device(dev);
if (!connected(ps)) {
- usb_unlock_device(dev);
- return -ENODEV;
+ ret = -ENODEV;
+ goto unlock;
+ }
+
+ /*
+ * Grab device's exclusive_access lock to prevent its driver from
+ * using this device while it is being accessed by us.
+ */
+ if (usb_excl_lock(ps->dev, 3, 1)) {
+ ret = -ERESTARTSYS;
+ goto unlock;
}
switch (cmd) {
@@ -2138,6 +2158,8 @@ static long usbdev_do_ioctl(struct file
ret = proc_disconnect_claim(ps, p);
break;
}
+ usb_excl_unlock(dev, 3);
+unlock:
usb_unlock_device(dev);
if (ret >= 0)
inode->i_atime = CURRENT_TIME;
Index: usb/drivers/usb/core/usb.c
===================================================================
--- usb.orig/drivers/usb/core/usb.c
+++ usb/drivers/usb/core/usb.c
@@ -442,6 +442,9 @@ struct usb_device *usb_alloc_dev(struct
dev->parent = parent;
INIT_LIST_HEAD(&dev->filelist);
+ spin_lock_init(&dev->excl_lock);
+ init_waitqueue_head(&dev->excl_wait);
+
#ifdef CONFIG_PM
pm_runtime_set_autosuspend_delay(&dev->dev,
usb_autosuspend_delay * 1000);
@@ -992,6 +995,64 @@ static void usb_debugfs_cleanup(void)
debugfs_remove(usb_debug_root);
}
+int usb_excl_lock(struct usb_device *dev, unsigned int type, bool interruptible)
+{
+ DECLARE_WAITQUEUE(waita, current);
+
+ add_wait_queue(&dev->excl_wait, &waita);
+ if (interruptible)
+ set_current_state(TASK_INTERRUPTIBLE);
+ else
+ set_current_state(TASK_UNINTERRUPTIBLE);
+
+ for (;;) {
+ spin_lock_irq(&dev->excl_lock);
+ switch (type) {
+ case 1: /* 1 - read */
+ case 2: /* 2 - write */
+ case 3: /* 3 - control: excludes both read and write */
+ if ((dev->excl_type & type) == 0) {
+ dev->excl_type |= type;
+ spin_unlock_irq(&dev->excl_lock);
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&dev->excl_wait, &waita);
+ return 0;
+ }
+ break;
+ default:
+ spin_unlock_irq(&dev->excl_lock);
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&dev->excl_wait, &waita);
+ return -EINVAL;
+ }
+ spin_unlock_irq(&dev->excl_lock);
+
+ if (interruptible) {
+ schedule();
+ if (signal_pending(current)) {
+ remove_wait_queue(&dev->excl_wait, &waita);
+ return 1;
+ }
+ set_current_state(TASK_INTERRUPTIBLE);
+ } else {
+ schedule();
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ }
+ }
+}
+EXPORT_SYMBOL_GPL(usb_excl_lock);
+
+void usb_excl_unlock(struct usb_device *dev, unsigned int type)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->excl_lock, flags);
+ dev->excl_type &= ~type;
+ wake_up(&dev->excl_wait);
+ spin_unlock_irqrestore(&dev->excl_lock, flags);
+}
+EXPORT_SYMBOL_GPL(usb_excl_unlock);
+
/*
* Init
*/
Index: usb/drivers/usb/storage/transport.c
===================================================================
--- usb.orig/drivers/usb/storage/transport.c
+++ usb/drivers/usb/storage/transport.c
@@ -601,9 +601,18 @@ void usb_stor_invoke_transport(struct sc
int need_auto_sense;
int result;
+ /*
+ * Grab device's exclusive_access lock to prevent libusb/usbfs from
+ * sending out a command in the middle of ours (if libusb sends a
+ * get_descriptor or something on EP0 after our CBW and before
+ * our CSW, some devices may malfunction.
+ */
+ usb_excl_lock(us->pusb_dev, 3, 0);
+
/* send the command to the transport layer */
scsi_set_resid(srb, 0);
result = us->transport(srb, us);
+ usb_excl_unlock(us->pusb_dev, 3);
/* if the command gets aborted by the higher layers, we need to
* short-circuit all other processing
@@ -712,8 +721,10 @@ Retry_Sense:
srb->cmd_len = 12;
/* issue the auto-sense command */
+ usb_excl_lock(us->pusb_dev, 3, 0);
scsi_set_resid(srb, 0);
temp_result = us->transport(us->srb, us);
+ usb_excl_unlock(us->pusb_dev, 3);
/* let's clean up right away */
scsi_eh_restore_cmnd(srb, &ses);
Index: usb/include/linux/usb.h
===================================================================
--- usb.orig/include/linux/usb.h
+++ usb/include/linux/usb.h
@@ -439,6 +439,13 @@ struct usb3_lpm_parameters {
* @speed: device speed: high/full/low (or error)
* @tt: Transaction Translator info; used with low/full speed dev, highspeed hub
* @ttport: device port on that tt hub
+ * @excl_wait: wait queue used to wait on @excl_type
+ * @excl_lock: spinlock guarding @excl_type
+ * @excl_type: exclusion lock type:
+ * 0 - unlocked
+ * 1 - locked for reads
+ * 2 - locked for writes
+ * 3 - locked for everything
* @toggle: one bit for each endpoint, with ([0] = IN, [1] = OUT) endpoints
* @parent: our hub, unless we're the root
* @bus: bus we're part of
@@ -508,6 +515,14 @@ struct usb_device {
struct usb_tt *tt;
int ttport;
+ /*
+ * This is our custom open-coded lock, similar to r/w locks in concept.
+ * It prevents usb-storage, usbfs, and /proc from simultaneous access.
+ */
+ wait_queue_head_t excl_wait;
+ spinlock_t excl_lock;
+ unsigned int excl_type;
+
unsigned int toggle[2];
struct usb_device *parent;
@@ -681,6 +696,10 @@ static inline bool usb_device_supports_l
/* for drivers using iso endpoints */
extern int usb_get_current_frame_number(struct usb_device *usb_dev);
+extern int usb_excl_lock(struct usb_device *dev, unsigned int type,
+ bool interruptible);
+extern void usb_excl_unlock(struct usb_device *dev, unsigned int type);
+
/* Sets up a group of bulk endpoints to support multiple stream IDs. */
extern int usb_alloc_streams(struct usb_interface *interface,
struct usb_host_endpoint **eps, unsigned int num_eps,
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
