Hello.
On 19-01-2013 0:22, Alan Stern wrote:
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
Assuming Greg ever agrees to merge this,
I doubt it now. :-)
I will insist on a few changes.
--- 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
Magic numbers like this are a terrible idea. Use an enumeration type
instead. And instead of using 0 or 1 for the "interruptible" argument,
use TASK_UNINTERRUPTIBLE or TASK_INTERRUPTIBLE. Better yet, have
separate "lock" and "lock_interruptible" functions.
OK, that's doable...
Furthermore, the locking in usbfs doesn't look right. A process should
be able to submit as many URBs as it wants, of whatever type, at any
time.
You mean USBDEVFS_SUBMITURB ioctl()? That's indeed an issue with 2.4 patch
(and mine, of course) -- it still accounts this ioctl() as exclusive and so
only permits one instance of URB. I should have documented it internally as a
side effect of the v1 patch but I'm not really familiar with usbfs, and so
documented only what was fixed by Pete back in 2006 (USBDEVFS_BULK).
> The locking should be by file handle, not by "read" vs. "write".
You mean we still don't allow more than one URB per file handle? That
would probably require a lock in the 'struct dev_state'...
Alan Stern
WBR, Sergei
--
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
