On Sat, May 29, 2004 at 03:23:09PM +0200, Oliver Neukum wrote:
> > Here I have this patch ported from a 2.4 verison of a patch by guy at
> > Dell ([EMAIL PROTECTED]), that uses the serialize semaphore in devio.c
> > and in usb-storage.
> >
> > This is needed on storage devices, where even sending a harmless
> > GET_DESCRIPTOR command at the wrong time will lead to a STALL condition,
> It seems to me that you should hold the semaphore until after the reset
> of the error case. You are dropping it too early.
Ok, how about this one?
--
Vojtech Pavlik
SuSE Labs, SuSE CR
[EMAIL PROTECTED], 2004-05-29 16:11:29+02:00, [EMAIL PROTECTED]
usb: Based on a 2.4 patch from [EMAIL PROTECTED], this patch
serializes usb storage and usbfs operation, so that usbfs
cannot disturb storage by seemingly harmless control reads.
Signed-off-by: Vojtech Pavlik <[EMAIL PROTECTED]>
core/devio.c | 3 +++
storage/transport.c | 20 ++++++++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c 2004-05-29 16:12:46 +02:00
+++ b/drivers/usb/core/devio.c 2004-05-29 16:12:46 +02:00
@@ -1184,6 +1184,8 @@
return -ENODEV;
}
+ down(&(ps->dev->serialize));
+
switch (cmd) {
case USBDEVFS_CONTROL:
snoop(&dev->dev, "%s: CONTROL\n", __FUNCTION__);
@@ -1280,6 +1282,7 @@
ret = proc_ioctl(ps, (void __user *) arg);
break;
}
+ up(&(ps->dev->serialize));
up(&dev->serialize);
if (ret >= 0)
inode->i_atime = CURRENT_TIME;
diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
--- a/drivers/usb/storage/transport.c 2004-05-29 16:12:46 +02:00
+++ b/drivers/usb/storage/transport.c 2004-05-29 16:12:46 +02:00
@@ -527,6 +527,15 @@
int need_auto_sense;
int result;
+ /*
+ * Grab device's serialize mutex to prevent /usbfs and others 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, and then we get a stall, we have trouble)
+ */
+
+ down (&(us->pusb_dev->serialize));
+
/* send the command to the transport layer */
srb->resid = 0;
result = us->transport(srb, us);
@@ -544,13 +553,13 @@
US_DEBUGP("-- transport indicates error, resetting\n");
us->transport_reset(us);
srb->result = DID_ERROR << 16;
- return;
+ goto End_Transport;
}
/* if the transport provided its own sense data, don't auto-sense */
if (result == USB_STOR_TRANSPORT_NO_SENSE) {
srb->result = SAM_STAT_CHECK_CONDITION;
- return;
+ goto End_Transport;
}
srb->result = SAM_STAT_GOOD;
@@ -676,7 +685,7 @@
if (!(us->flags & US_FL_SCM_MULT_TARG))
us->transport_reset(us);
srb->result = DID_ERROR << 16;
- return;
+ goto End_Transport;
}
US_DEBUGP("-- Result from auto-sense is %d\n", temp_result);
@@ -708,7 +717,7 @@
srb->sense_buffer[0] = 0x0;
}
}
- return;
+ goto End_Transport;
/* abort processing: the bulk-only transport requires a reset
* following an abort */
@@ -716,6 +725,9 @@
srb->result = DID_ABORT << 16;
if (us->protocol == US_PR_BULK)
us->transport_reset(us);
+
+ End_Transport:
+ up(&(us->pusb_dev->serialize));
}
/* Stop the current URB transfer */
Signed-off-by: Vojtech Pavlik <[EMAIL PROTECTED]>
