[PATCH] USB: prevent overlapping access by usb-storage and usbfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.28 patch by Pete Zaitcev -- which I even had to
reconstruct as I have never found it in its final  form.  That patch dates back
to 2004 and it unfortunately wasn't applied to 2.6 branch in the same form back
then (it was applied in another form and then immediately reverted). Despite 8+
years passing from that moment, the vendors didn't stop producing USB devices
that require this 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 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...

 drivers/usb/core/devices.c      |   13 +++++++++++--
 drivers/usb/core/devio.c        |    7 +++++++
 drivers/usb/core/usb.c          |    2 ++
 drivers/usb/storage/transport.c |   11 +++++++++++
 include/linux/usb.h             |    4 ++++
 5 files changed, 35 insertions(+), 2 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
+	 * devio from using this device while we are accessing it.
+	 */
+	mutex_lock(&dev->exclusive_access);
+
 	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:
+	mutex_unlock(&dev->exclusive_access);
 	return start;
 }
 
Index: usb/drivers/usb/core/devio.c
===================================================================
--- usb.orig/drivers/usb/core/devio.c
+++ usb/drivers/usb/core/devio.c
@@ -1983,6 +1983,12 @@ static long usbdev_do_ioctl(struct file 
 		return -ENODEV;
 	}
 
+	/*
+	 * Grab device's exclusive_access mutex to prevent its driver from
+	 * using this device while it is being accessed by us.
+	 */
+	mutex_lock(&dev->exclusive_access);
+
 	switch (cmd) {
 	case USBDEVFS_CONTROL:
 		snoop(&dev->dev, "%s: CONTROL\n", __func__);
@@ -2138,6 +2144,7 @@ static long usbdev_do_ioctl(struct file 
 		ret = proc_disconnect_claim(ps, p);
 		break;
 	}
+	mutex_unlock(&dev->exclusive_access);
 	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,8 @@ struct usb_device *usb_alloc_dev(struct 
 	dev->parent = parent;
 	INIT_LIST_HEAD(&dev->filelist);
 
+	mutex_init(&dev->exclusive_access);
+
 #ifdef	CONFIG_PM
 	pm_runtime_set_autosuspend_delay(&dev->dev,
 			usb_autosuspend_delay * 1000);
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 mutex to prevent libusb/usbfs from
+	 * sending out a command in the middle of ours (if libusb sends a
+	 * get_descriptor or something on pipe 0 after our CBW and before
+	 * our CSW, some devices may malfunction.
+	 */
+	mutex_lock(&us->pusb_dev->exclusive_access);
+
 	/* send the command to the transport layer */
 	scsi_set_resid(srb, 0);
 	result = us->transport(srb, us);
+	mutex_unlock(&us->pusb_dev->exclusive_access);
 
 	/* 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 */
+		mutex_lock(&us->pusb_dev->exclusive_access);
 		scsi_set_resid(srb, 0);
 		temp_result = us->transport(us->srb, us);
+		mutex_unlock(&us->pusb_dev->exclusive_access);
 
 		/* 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,8 @@ 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
+ * @exclusive_access: mutex to prevent driver & proc accesses from overlapping
+ *	commands at device
  * @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 +510,8 @@ struct usb_device {
 	struct usb_tt	*tt;
 	int		ttport;
 
+	struct mutex exclusive_access;
+
 	unsigned int toggle[2];
 
 	struct usb_device *parent;
--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux