Re: BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

Re: BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[James Bottomley]

On Tue, 2008-07-29 at 17:06 -0700, Matthew Frost wrote:
> James and co., 
> 
> Bug report: regression in 2.6.27-rc1 -- scsi WRT usb-storage
> Origin: Commit de72aa4c2b82a6cffe15d86a8d391ded4fb57602, "[SCSI] erase
> invalid data returned by device"
> Location: drivers/scsi_lib.c
> Device: Secure Digital HC 4GB card in USB 2.0 card reader

Actually, this is a USB issue ... I've added the correct cc's

James


> Summary:
> Between 2.6.26-rc9-git10 and rc9-git11, the kernel stopped properly
> recognizing my USB-card-reader-mounted SDHC 4GB card.  The problem seems to
> come up between usb-storage recognizing the device, and the handoff to scsi
> to read the disk properties.  The bug remains in 2.6.26 and 2.6.27-rc1;
> kernels before 2.6.26-rc9-git10 are unaffected.  On the same EHCI controller,
> an old-spec SD 1GB card and its reader are unaffected by this bug.
> 
> The normal recognition and initialization of the device (from rc9-git9 dmesg)
> goes like this:
> 
> PCI: Setting latency timer of device 0000:00:1d.7 to 64
> ehci_hcd 0000:00:1d.7: EHCI Host Controller
> ehci_hcd 0000:00:1d.7: new USB bus registered, assigned bus number 1
> PCI: cache line size of 128 is not supported by device 0000:00:1d.7
> ehci_hcd 0000:00:1d.7: irq 23, io mem 0xe4000000
> ehci_hcd 0000:00:1d.7: USB 2.0 started, EHCI 1.00, driver 10 Dec 2004
> usb usb1: configuration #1 chosen from 1 choice
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 6 ports detected
> usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
> usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb1: Product: EHCI Host Controller
> usb usb1: Manufacturer: Linux 2.6.26-rc9-git9 ehci_hcd
> usb usb1: SerialNumber: 0000:00:1d.7
> --%<--
> usb 1-1: new high speed USB device using ehci_hcd and address 2
> usb 1-1: configuration #1 chosen from 1 choice
> usb 1-1: New USB device found, idVendor=058f, idProduct=6331
> usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 1-1: Product: Mass Storage Device
> usb 1-1: Manufacturer: Generic
> usb 1-1: SerialNumber: 058F091111B
> --%<--
> usb 1-2: new high speed USB device using ehci_hcd and address 3
> usb 1-2: configuration #1 chosen from 1 choice
> usb 1-2: New USB device found, idVendor=090c, idProduct=6000
> usb 1-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 1-2: Product: USB2.0 Card Reader
> usb 1-2: Manufacturer: Generic       ,   .
> usb 1-2: SerialNumber: 12345678901234567890
> --%<--
> Initializing USB Mass Storage driver...
> scsi4 : SCSI emulation for USB Mass Storage devices
> usb-storage: device found at 2
> usb-storage: waiting for device to settle before scanning
> scsi5 : SCSI emulation for USB Mass Storage devices
> usb-storage: device found at 3
> usb-storage: waiting for device to settle before scanning
> --%<--
> usbcore: registered new interface driver usb-storage
> USB Mass Storage support registered.
> usbcore: registered new interface driver libusual
> --%<--
> usb-storage: device scan complete
> scsi 4:0:0:0: Direct-Access     Multi    Flash Reader     1.00 PQ: 0 ANSI: 0
> usb-storage: device scan complete
> scsi 5:0:0:0: Direct-Access     Generic                   6000 PQ: 0 ANSI: 0
> CCS
> sd 4:0:0:0: [sde] 1953792 512-byte hardware sectors (1000 MB)
> sd 4:0:0:0: [sde] Write Protect is off
> sd 4:0:0:0: [sde] Mode Sense: 03 00 00 00
> sd 4:0:0:0: [sde] Assuming drive cache: write through
> sd 4:0:0:0: [sde] 1953792 512-byte hardware sectors (1000 MB)
> sd 4:0:0:0: [sde] Write Protect is off
> sd 4:0:0:0: [sde] Mode Sense: 03 00 00 00
> sd 4:0:0:0: [sde] Assuming drive cache: write through
>  sde: sde1
> sd 4:0:0:0: [sde] Attached SCSI removable disk
> sd 4:0:0:0: Attached scsi generic sg6 type 0
> sd 5:0:0:0: [sdf] 7861248 512-byte hardware sectors (4025 MB)
> sd 5:0:0:0: [sdf] Write Protect is off
> sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> sd 5:0:0:0: [sdf] Assuming drive cache: write through
> sd 5:0:0:0: [sdf] 7861248 512-byte hardware sectors (4025 MB)
> sd 5:0:0:0: [sdf] Write Protect is off
> sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> sd 5:0:0:0: [sdf] Assuming drive cache: write through
>  sdf: sdf1
> sd 5:0:0:0: [sdf] Attached SCSI removable disk
> sd 5:0:0:0: Attached scsi generic sg7 type 0
> 
> (sde is the 1GB SD card/reader, sdf is the 4GB SDHC card/reader.)
> 
> After the bug, initialization looks the same until the device scan finishes. 
> Handoff to scsi produces the following (taken from rc9-git12):
> 
> usb-storage: device scan complete
> scsi 4:0:0:0: Direct-Access     Multi    Flash Reader     1.00 PQ: 0 ANSI: 0
> usb-storage: device scan complete
> scsi 5:0:0:0: Direct-Access     Generic                   6000 PQ: 0 ANSI: 0
> CCS
> sd 4:0:0:0: [sde] 1953792 512-byte hardware sectors (1000 MB)
> sd 4:0:0:0: [sde] Write Protect is off
> sd 4:0:0:0: [sde] Mode Sense: 03 00 00 00
> sd 4:0:0:0: [sde] Assuming drive cache: write through
> sd 4:0:0:0: [sde] 1953792 512-byte hardware sectors (1000 MB)
> sd 4:0:0:0: [sde] Write Protect is off
> sd 4:0:0:0: [sde] Mode Sense: 03 00 00 00
> sd 4:0:0:0: [sde] Assuming drive cache: write through
>  sde: sde1
> sd 4:0:0:0: [sde] Attached SCSI removable disk
> sd 4:0:0:0: Attached scsi generic sg6 type 0
> sd 5:0:0:0: [sdf] Sector size 0 reported, assuming 512.
> sd 5:0:0:0: [sdf] 1 512-byte hardware sectors (0 MB)
> sd 5:0:0:0: [sdf] Write Protect is off
> sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> sd 5:0:0:0: [sdf] Assuming drive cache: write through
> sd 5:0:0:0: [sdf] Sector size 0 reported, assuming 512.
> sd 5:0:0:0: [sdf] 1 512-byte hardware sectors (0 MB)
> sd 5:0:0:0: [sdf] Write Protect is off
> sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> sd 5:0:0:0: [sdf] Assuming drive cache: write through
>  sdf: sdf1
>  sdf: p1 exceeds device capacity
> sd 5:0:0:0: [sdf] Attached SCSI removable disk
> sd 5:0:0:0: Attached scsi generic sg7 type 0
> 
> And after boot, the following set of errors recurs until device removal.
> Device removal, in this case, was necessary to get LILO to update (-git12
> again):
> 
> attempt to access beyond end of device
> sdf: rw=0, want=8200, limit=1
> Buffer I/O error on device sdf1, logical block 0
> attempt to access beyond end of device
> sdf: rw=0, want=8208, limit=1
> Buffer I/O error on device sdf1, logical block 1
> attempt to access beyond end of device
> sdf: rw=0, want=8216, limit=1
> Buffer I/O error on device sdf1, logical block 2
> attempt to access beyond end of device
> sdf: rw=0, want=8224, limit=1
> Buffer I/O error on device sdf1, logical block 3
> attempt to access beyond end of device
> sdf: rw=0, want=8200, limit=1
> Buffer I/O error on device sdf1, logical block 0
> attempt to access beyond end of device
> sdf: rw=0, want=8200, limit=1
> Buffer I/O error on device sdf1, logical block 0
> attempt to access beyond end of device
> sdf: rw=0, want=8208, limit=1
> Buffer I/O error on device sdf1, logical block 1
> attempt to access beyond end of device
> sdf: rw=0, want=8216, limit=1
> Buffer I/O error on device sdf1, logical block 2
> attempt to access beyond end of device
> sdf: rw=0, want=8224, limit=1
> Buffer I/O error on device sdf1, logical block 3
> attempt to access beyond end of device
> sdf: rw=0, want=8200, limit=1
> Buffer I/O error on device sdf1, logical block 0
> attempt to access beyond end of device
> sdf: rw=0, want=8200, limit=1
> Buffer I/O error on device sdf1, logical block 0
> attempt to access beyond end of device
> sdf: rw=0, want=8208, limit=1
> Buffer I/O error on device sdf1, logical block 1
> attempt to access beyond end of device
> sdf: rw=0, want=8216, limit=1
> Buffer I/O error on device sdf1, logical block 2
> attempt to access beyond end of device
> sdf: rw=0, want=8224, limit=1
> Buffer I/O error on device sdf1, logical block 3
> attempt to access beyond end of device
> sdf: rw=0, want=8200, limit=1
> Buffer I/O error on device sdf1, logical block 0
> usb 1-2: USB disconnect, address 3
> 
> I'm not well-versed in git, so I looked through the diff between rc9-git10
> and rc9-git11.  This is what looks like the offending addition:
> 
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index a82d2fe..cbf55d5 100644
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -207,6 +207,15 @@ int scsi_execute(struct scsi_device *sdev, const
> unsigned char *cmd,
> >  	 */
> >  	blk_execute_rq(req->q, NULL, req, 1);
> >  
> > +	/*
> > +	 * Some devices (USB mass-storage in particular) may transfer
> > +	 * garbage data together with a residue indicating that the data
> > +	 * is invalid.  Prevent the garbage from being misinterpreted
> > +	 * and prevent security leaks by zeroing out the excess data.
> > +	 */
> > +	if (unlikely(req->data_len > 0 && req->data_len <= bufflen))
> > +		memset(buffer + (bufflen - req->data_len), 0, req->data_len);
> > +
> >  	ret = req->errors;
> >   out:
> >  	blk_put_request(req);
> 
> I looked through the online git log, and it's part of commit
> de72aa4c2b82a6cffe15d86a8d391ded4fb57602, under "[SCSI] erase invalid data
> returned by device".  Reverting this part by itself restores the correct
> handling of the device in 2.6.27-rc1.
> 
> Perhaps there's a better way to do what this patch is trying to do, because
> it seems to be erasing the valid data returned by my device.  If you want me
> to test out options, I'd be glad to, since I have the "offending" hardware. 
> Specifics: A-Data 4GB TurboSD Class 6 SDHC card, 4GB, in a Rosewill RSD-CR102
> card reader.
> 
> (Please include me on replies, I've stopped being a subscriber.)
> 
> Thanks!
> 
> Matthew Frost
> artusemrys sbcglobal net

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1594 bytes --]

On Tue, Jul 29, 2008 at 11:08:42PM -0500, James Bottomley wrote:
> On Tue, 2008-07-29 at 17:06 -0700, Matthew Frost wrote:
> > James and co., 
> > 
> > Bug report: regression in 2.6.27-rc1 -- scsi WRT usb-storage
> > Origin: Commit de72aa4c2b82a6cffe15d86a8d391ded4fb57602, "[SCSI] erase
> > invalid data returned by device"
> > Location: drivers/scsi_lib.c
> > Device: Secure Digital HC 4GB card in USB 2.0 card reader
> 
> Actually, this is a USB issue ... I've added the correct cc's

Well, it's both a SCSI and USB issue.

The patch in question clears sections of a data buffer that a device
reports as invalid.  Basically, the usb storage spec allows devices to
transfer "garbage" data into buffers; instead of leaving the data there
(which could be leakage from something sensitive), the SCSI core now zeros
out the section of buffers that are reported as 'unused' (aka 'residue').

It does this for all devices, not just USB ones.  USB devices, however,
seem especially prone to not reporting this 'residue' correctly.

Honestly, given the problems this has caused, and the (apparently)
relatively high number of devices that don't report residue correctly, I'm
starting to seriously think this should be reverted.

Actually, I'm seriously starting to think that US_FL_IGNORE_RESIDUE should
just become a sysfs parameter which defaults to the 'ignore' state...

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

You suck Stef.
					-- Greg 
User Friendly, 11/29/97

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Tue, 29 Jul 2008, James Bottomley wrote:

> > Summary:
> > Between 2.6.26-rc9-git10 and rc9-git11, the kernel stopped properly
> > recognizing my USB-card-reader-mounted SDHC 4GB card.  The problem seems to
> > come up between usb-storage recognizing the device, and the handoff to scsi
> > to read the disk properties.  The bug remains in 2.6.26 and 2.6.27-rc1;
> > kernels before 2.6.26-rc9-git10 are unaffected.  On the same EHCI controller,
> > an old-spec SD 1GB card and its reader are unaffected by this bug.

> > After the bug, initialization looks the same until the device scan finishes. 
> > Handoff to scsi produces the following (taken from rc9-git12):

> > sd 5:0:0:0: [sdf] Sector size 0 reported, assuming 512.
> > sd 5:0:0:0: [sdf] 1 512-byte hardware sectors (0 MB)
> > sd 5:0:0:0: [sdf] Write Protect is off
> > sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> > sd 5:0:0:0: [sdf] Assuming drive cache: write through
> > sd 5:0:0:0: [sdf] Sector size 0 reported, assuming 512.
> > sd 5:0:0:0: [sdf] 1 512-byte hardware sectors (0 MB)
> > sd 5:0:0:0: [sdf] Write Protect is off
> > sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> > sd 5:0:0:0: [sdf] Assuming drive cache: write through
> >  sdf: sdf1
> >  sdf: p1 exceeds device capacity
> > sd 5:0:0:0: [sdf] Attached SCSI removable disk
> > sd 5:0:0:0: Attached scsi generic sg7 type 0

The problem will most likely be fixed by this patch:

	http://marc.info/?l=linux-usb&m=121734710306509&w=2

Alan Stern

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Tue, 29 Jul 2008, Matthew Dharm wrote:

> Actually, I'm seriously starting to think that US_FL_IGNORE_RESIDUE should
> just become a sysfs parameter which defaults to the 'ignore' state...

We have to be careful; there definitely are devices out there which 
need to use the Residue.

Alan Stern

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[James Bottomley]

On Wed, 2008-07-30 at 10:17 -0400, Alan Stern wrote:
> On Tue, 29 Jul 2008, Matthew Dharm wrote:
> 
> > Actually, I'm seriously starting to think that US_FL_IGNORE_RESIDUE should
> > just become a sysfs parameter which defaults to the 'ignore' state...
> 
> We have to be careful; there definitely are devices out there which 
> need to use the Residue.

This is sort of a tradeoff ... there was one complaint I saw where a
device turned read only without the fix idenitifed in this report.
Devices broken by the fix are definitely crawling out of the woodwork
now.  Either this patch needs to be reverted or a new fix needs to be
applied soon (and to stable).

James

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Wed, 30 Jul 2008, James Bottomley wrote:

> On Wed, 2008-07-30 at 10:17 -0400, Alan Stern wrote:
> > On Tue, 29 Jul 2008, Matthew Dharm wrote:
> > 
> > > Actually, I'm seriously starting to think that US_FL_IGNORE_RESIDUE should
> > > just become a sysfs parameter which defaults to the 'ignore' state...
> > 
> > We have to be careful; there definitely are devices out there which 
> > need to use the Residue.
> 
> This is sort of a tradeoff ... there was one complaint I saw where a
> device turned read only without the fix idenitifed in this report.
> Devices broken by the fix are definitely crawling out of the woodwork
> now.  Either this patch needs to be reverted or a new fix needs to be
> applied soon (and to stable).

I would expect this patch:

	http://marc.info/?l=linux-usb&m=121734710306509&w=2

to take care of most of the problems.  But it hasn't yet been merged,
and it can't go into -stable until then.

Alan Stern

Re: BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Matthew Frost]

--- Alan Stern <stern@rowland.harvard.edu> wrote:

> On Tue, 29 Jul 2008, James Bottomley wrote:
> 
> > > Summary:
> > > Between 2.6.26-rc9-git10 and rc9-git11, the kernel stopped properly
> > > recognizing my USB-card-reader-mounted SDHC 4GB card.  The problem
> seems to
> > > come up between usb-storage recognizing the device, and the handoff to
> scsi
> > > to read the disk properties.  The bug remains in 2.6.26 and 2.6.27-rc1;
> > > kernels before 2.6.26-rc9-git10 are unaffected.  On the same EHCI
> controller,
> > > an old-spec SD 1GB card and its reader are unaffected by this bug.
> 
> > > After the bug, initialization looks the same until the device scan
> finishes. 
> > > Handoff to scsi produces the following (taken from rc9-git12):
> 
> > > sd 5:0:0:0: [sdf] Sector size 0 reported, assuming 512.
> > > sd 5:0:0:0: [sdf] 1 512-byte hardware sectors (0 MB)
> > > sd 5:0:0:0: [sdf] Write Protect is off
> > > sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> > > sd 5:0:0:0: [sdf] Assuming drive cache: write through
> > > sd 5:0:0:0: [sdf] Sector size 0 reported, assuming 512.
> > > sd 5:0:0:0: [sdf] 1 512-byte hardware sectors (0 MB)
> > > sd 5:0:0:0: [sdf] Write Protect is off
> > > sd 5:0:0:0: [sdf] Mode Sense: 4b 00 00 08
> > > sd 5:0:0:0: [sdf] Assuming drive cache: write through
> > >  sdf: sdf1
> > >  sdf: p1 exceeds device capacity
> > > sd 5:0:0:0: [sdf] Attached SCSI removable disk
> > > sd 5:0:0:0: Attached scsi generic sg7 type 0
> 
> The problem will most likely be fixed by this patch:
> 
> 	http://marc.info/?l=linux-usb&m=121734710306509&w=2
> 
> Alan Stern
> 
> 

Thanks.  Giving it a try now.

Matt Frost

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Douglas Gilbert]

James Bottomley wrote:
> On Wed, 2008-07-30 at 10:17 -0400, Alan Stern wrote:
>> On Tue, 29 Jul 2008, Matthew Dharm wrote:
>>
>>> Actually, I'm seriously starting to think that US_FL_IGNORE_RESIDUE should
>>> just become a sysfs parameter which defaults to the 'ignore' state...
>> We have to be careful; there definitely are devices out there which 
>> need to use the Residue.
> 
> This is sort of a tradeoff ... there was one complaint I saw where a
> device turned read only without the fix idenitifed in this report.
> Devices broken by the fix are definitely crawling out of the woodwork
> now.  Either this patch needs to be reverted or a new fix needs to be
> applied soon (and to stable).

The patches to fix this that I have tried do not apply
cleanly to lk 2.6.26 (and break during compile if forced:
"us->fflags" is not defined).

Is there a lk 2.6.26 patch available?

Doug Gilbert

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Wed, 30 Jul 2008, Douglas Gilbert wrote:

> James Bottomley wrote:
> > On Wed, 2008-07-30 at 10:17 -0400, Alan Stern wrote:
> >> On Tue, 29 Jul 2008, Matthew Dharm wrote:
> >>
> >>> Actually, I'm seriously starting to think that US_FL_IGNORE_RESIDUE should
> >>> just become a sysfs parameter which defaults to the 'ignore' state...
> >> We have to be careful; there definitely are devices out there which 
> >> need to use the Residue.
> > 
> > This is sort of a tradeoff ... there was one complaint I saw where a
> > device turned read only without the fix idenitifed in this report.
> > Devices broken by the fix are definitely crawling out of the woodwork
> > now.  Either this patch needs to be reverted or a new fix needs to be
> > applied soon (and to stable).
> 
> The patches to fix this that I have tried do not apply
> cleanly to lk 2.6.26 (and break during compile if forced:
> "us->fflags" is not defined).
> 
> Is there a lk 2.6.26 patch available?

Sorry about that; my patches are against the USB development tree and
I tend to forget to redo them against the vanilla kernel.  Below is a
patch against 2.6.26.  Or you can just edit the original patch and
change the occurrences of "fflags" to "flags".

Alan Stern


Index: 2.6.26/drivers/usb/storage/transport.c
===================================================================
--- 2.6.26.orig/drivers/usb/storage/transport.c
+++ 2.6.26/drivers/usb/storage/transport.c
@@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
 
 	/* try to compute the actual residue, based on how much data
 	 * was really transferred and what the device tells us */
-	if (residue) {
-		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
+	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
+
+		/* Heuristically detect devices that generate bogus residues
+		 * by seeing what happens with INQUIRY and READ CAPACITY
+		 * commands.
+		 */
+		if (bcs->Status == US_BULK_STAT_OK &&
+				scsi_get_resid(srb) == 0 &&
+					((srb->cmnd[0] == INQUIRY &&
+						transfer_length == 36) ||
+					(srb->cmnd[0] == READ_CAPACITY &&
+						transfer_length == 8))) {
+			us->flags |= US_FL_IGNORE_RESIDUE;
+
+		} else {
 			residue = min(residue, transfer_length);
 			scsi_set_resid(srb, max(scsi_get_resid(srb),
 			                                       (int) residue));

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Pete Zaitcev]

On Wed, 30 Jul 2008 17:00:10 -0400 (EDT), Alan Stern <stern@rowland.harvard.edu> wrote:

> +++ 2.6.26/drivers/usb/storage/transport.c
> @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
> +		/* Heuristically detect devices that generate bogus residues
> +		 * by seeing what happens with INQUIRY and READ CAPACITY
> +		 * commands.
> +		 */
> +		if (bcs->Status == US_BULK_STAT_OK &&
> +				scsi_get_resid(srb) == 0 &&
> +					((srb->cmnd[0] == INQUIRY &&
> +						transfer_length == 36) ||
> +					(srb->cmnd[0] == READ_CAPACITY &&
> +						transfer_length == 8))) {
> +			us->flags |= US_FL_IGNORE_RESIDUE;

Why do you do this for INQUIRY and READ_CAPACITY only?
Why not do it for any command?

-- Pete

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Wed, 30 Jul 2008, Pete Zaitcev wrote:

> On Wed, 30 Jul 2008 17:00:10 -0400 (EDT), Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > +++ 2.6.26/drivers/usb/storage/transport.c
> > @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
> > +		/* Heuristically detect devices that generate bogus residues
> > +		 * by seeing what happens with INQUIRY and READ CAPACITY
> > +		 * commands.
> > +		 */
> > +		if (bcs->Status == US_BULK_STAT_OK &&
> > +				scsi_get_resid(srb) == 0 &&
> > +					((srb->cmnd[0] == INQUIRY &&
> > +						transfer_length == 36) ||
> > +					(srb->cmnd[0] == READ_CAPACITY &&
> > +						transfer_length == 8))) {
> > +			us->flags |= US_FL_IGNORE_RESIDUE;
> 
> Why do you do this for INQUIRY and READ_CAPACITY only?
> Why not do it for any command?

Because those are the only two commands for which I'm reasonably
certain the device should never return a nonzero residue with Okay
status.  For other commands there might be a valid positive residue --
although if there is then the device should also return Check Condition
status (the spec is unclear on this point).

Actually I'm a little concerned about the READ CAPACITY command; it's
conceivable that a removable-media device with no media present might
return a valid positive residue.  However the chance of something like
that happening and causing a problem is probably sufficiently small
that we can ignore it.

Alan Stern

Re: BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Pete Zaitcev]

On Wed, 30 Jul 2008 17:28:49 -0400 (EDT), Alan Stern <stern@rowland.harvard.edu> wrote:

> > > +		if (bcs->Status == US_BULK_STAT_OK &&
> > > +				scsi_get_resid(srb) == 0 &&
> > > +					((srb->cmnd[0] == INQUIRY &&
> > > +						transfer_length == 36) ||
> > > +					(srb->cmnd[0] == READ_CAPACITY &&
> > > +						transfer_length == 8))) {
> > > +			us->flags |= US_FL_IGNORE_RESIDUE;
> > 
> > Why do you do this for INQUIRY and READ_CAPACITY only?
> > Why not do it for any command?
> 
> Because those are the only two commands for which I'm reasonably
> certain the device should never return a nonzero residue with Okay
> status.  For other commands there might be a valid positive residue --
> although if there is then the device should also return Check Condition
> status (the spec is unclear on this point).

Perhaps I misunderstand how our SCSI stack works. The code in ub is
simpler, it deals with 3 values at the end of transfer:
  Lasked is how much we asked for
  Lgot   is how much was transferred
  Lresid is the reported residue

So, ub checks if the following is true:
  Lasked = Lgot + Lresid

If device fails this check, you can assume that it's just not set up
to report the residue correctly and so, the danger of valid residue
that you outlined becomes rather academic.

The reason I do it this way is, I've seen a device which reported
a correct residue until the first long read, and then residue was
miscalculated due to a 16-bit wrap (it did transfer right data though).
I think it's one of those which are explicitly blacklisted these
days, but I cannot remember.

-- Pete

Re: BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Wed, 30 Jul 2008, Pete Zaitcev wrote:

> Perhaps I misunderstand how our SCSI stack works. The code in ub is
> simpler, it deals with 3 values at the end of transfer:
>   Lasked is how much we asked for
>   Lgot   is how much was transferred
>   Lresid is the reported residue
> 
> So, ub checks if the following is true:
>   Lasked = Lgot + Lresid
> 
> If device fails this check, you can assume that it's just not set up
> to report the residue correctly and so, the danger of valid residue
> that you outlined becomes rather academic.

This algorithm is wrong.  See the description under Case (4) or (5) in 
6.7.2 of the Bulk-Only spec:

	The device may send fill data to pad up to a total of
	dCBWDataTransferLength.

So it's legal to have Lgot == Lasked and Lresid > 0.  There are devices 
which really do this.

(It may seem pointless to add the padding.  However for reasons that 
aren't clear, the spec requires the device to STALL the bulk-in 
endpoint if the padding isn't present -- and many devices don't like to 
STALL bulk endpoints.  Similar reasoning applies to the case of OUT 
transfers.)

> The reason I do it this way is, I've seen a device which reported
> a correct residue until the first long read, and then residue was
> miscalculated due to a 16-bit wrap (it did transfer right data though).
> I think it's one of those which are explicitly blacklisted these
> days, but I cannot remember.

I hope it's blacklisted!

Alan Stern

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Matthew Frost]

Alan Stern wrote:
> > The patches to fix this that I have tried do not apply
> > cleanly to lk 2.6.26 (and break during compile if forced:
> > "us->fflags" is not defined).
> > 
> > Is there a lk 2.6.26 patch available?
> 
> Sorry about that; my patches are against the USB development tree and
> I tend to forget to redo them against the vanilla kernel.  Below is a
> patch against 2.6.26.  Or you can just edit the original patch and
> change the occurrences of "fflags" to "flags".
> 
> Alan Stern
> 
> 
> Index: 2.6.26/drivers/usb/storage/transport.c
> ===================================================================
> --- 2.6.26.orig/drivers/usb/storage/transport.c
> +++ 2.6.26/drivers/usb/storage/transport.c
> @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
>  
>  	/* try to compute the actual residue, based on how much data
>  	 * was really transferred and what the device tells us */
> -	if (residue) {
> -		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
> +	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
> +
> +		/* Heuristically detect devices that generate bogus residues
> +		 * by seeing what happens with INQUIRY and READ CAPACITY
> +		 * commands.
> +		 */
> +		if (bcs->Status == US_BULK_STAT_OK &&
> +				scsi_get_resid(srb) == 0 &&
> +					((srb->cmnd[0] == INQUIRY &&
> +						transfer_length == 36) ||
> +					(srb->cmnd[0] == READ_CAPACITY &&
> +						transfer_length == 8))) {
> +			us->flags |= US_FL_IGNORE_RESIDUE;
> +
> +		} else {
>  			residue = min(residue, transfer_length);
>  			scsi_set_resid(srb, max(scsi_get_resid(srb),
>  			                                       (int) residue));
> 
> 

Thanks!  I've been trying to fix it manually, and it wouldn't work.  Trying
this version now.  Let's see if this fixes my problem.

Matt Frost

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Matthew Frost]

--- Matthew Frost <artusemrys@sbcglobal.net> wrote:

> Alan Stern wrote:
> > > The patches to fix this that I have tried do not apply
> > > cleanly to lk 2.6.26 (and break during compile if forced:
> > > "us->fflags" is not defined).
> > > 
> > > Is there a lk 2.6.26 patch available?
> > 
> > Sorry about that; my patches are against the USB development tree and
> > I tend to forget to redo them against the vanilla kernel.  Below is a
> > patch against 2.6.26.  Or you can just edit the original patch and
> > change the occurrences of "fflags" to "flags".
> > 
> > Alan Stern
> > 
> > 
> > Index: 2.6.26/drivers/usb/storage/transport.c
> > ===================================================================
> > --- 2.6.26.orig/drivers/usb/storage/transport.c
> > +++ 2.6.26/drivers/usb/storage/transport.c
> > @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
> >  
> >  	/* try to compute the actual residue, based on how much data
> >  	 * was really transferred and what the device tells us */
> > -	if (residue) {
> > -		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
> > +	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
> > +
> > +		/* Heuristically detect devices that generate bogus residues
> > +		 * by seeing what happens with INQUIRY and READ CAPACITY
> > +		 * commands.
> > +		 */
> > +		if (bcs->Status == US_BULK_STAT_OK &&
> > +				scsi_get_resid(srb) == 0 &&
> > +					((srb->cmnd[0] == INQUIRY &&
> > +						transfer_length == 36) ||
> > +					(srb->cmnd[0] == READ_CAPACITY &&
> > +						transfer_length == 8))) {
> > +			us->flags |= US_FL_IGNORE_RESIDUE;
> > +
> > +		} else {
> >  			residue = min(residue, transfer_length);
> >  			scsi_set_resid(srb, max(scsi_get_resid(srb),
> >  			                                       (int) residue));
> > 
> > 
> 
> Thanks!  I've been trying to fix it manually, and it wouldn't work.  Trying
> this version now.  Let's see if this fixes my problem.
> 
> Matt Frost
> 

Tested under two distributions, this patch restores correct functionality to
my hardware.  Thank you very much!

Matt

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Douglas Gilbert]

Matthew Frost wrote:
> --- Matthew Frost <artusemrys@sbcglobal.net> wrote:
> 
>> Alan Stern wrote:
>>>> The patches to fix this that I have tried do not apply
>>>> cleanly to lk 2.6.26 (and break during compile if forced:
>>>> "us->fflags" is not defined).
>>>>
>>>> Is there a lk 2.6.26 patch available?
>>> Sorry about that; my patches are against the USB development tree and
>>> I tend to forget to redo them against the vanilla kernel.  Below is a
>>> patch against 2.6.26.  Or you can just edit the original patch and
>>> change the occurrences of "fflags" to "flags".
>>>
>>> Alan Stern
>>>
>>>
>>> Index: 2.6.26/drivers/usb/storage/transport.c
>>> ===================================================================
>>> --- 2.6.26.orig/drivers/usb/storage/transport.c
>>> +++ 2.6.26/drivers/usb/storage/transport.c
>>> @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
>>>  
>>>  	/* try to compute the actual residue, based on how much data
>>>  	 * was really transferred and what the device tells us */
>>> -	if (residue) {
>>> -		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
>>> +	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
>>> +
>>> +		/* Heuristically detect devices that generate bogus residues
>>> +		 * by seeing what happens with INQUIRY and READ CAPACITY
>>> +		 * commands.
>>> +		 */
>>> +		if (bcs->Status == US_BULK_STAT_OK &&
>>> +				scsi_get_resid(srb) == 0 &&
>>> +					((srb->cmnd[0] == INQUIRY &&
>>> +						transfer_length == 36) ||
>>> +					(srb->cmnd[0] == READ_CAPACITY &&
>>> +						transfer_length == 8))) {
>>> +			us->flags |= US_FL_IGNORE_RESIDUE;
>>> +
>>> +		} else {
>>>  			residue = min(residue, transfer_length);
>>>  			scsi_set_resid(srb, max(scsi_get_resid(srb),
>>>  			                                       (int) residue));
>>>
>>>
>> Thanks!  I've been trying to fix it manually, and it wouldn't work.  Trying
>> this version now.  Let's see if this fixes my problem.
>>
>> Matt Frost

The above patch made my USB key:
   Alcor Micro Corp. Transcend JetFlash 110 USB 2.0 Flash Drive (2GB)
usable in a pretty clean lk 2.6.26 .

Thanks
Doug Gilbert

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Matthew Frost]

--- Matthew Frost <artusemrys@yahoo.com> wrote:

> --- Matthew Frost <artusemrys@sbcglobal.net> wrote:
> 
> > Alan Stern wrote:
> > > > The patches to fix this that I have tried do not apply
> > > > cleanly to lk 2.6.26 (and break during compile if forced:
> > > > "us->fflags" is not defined).
> > > > 
> > > > Is there a lk 2.6.26 patch available?
> > > 
> > > Sorry about that; my patches are against the USB development tree and
> > > I tend to forget to redo them against the vanilla kernel.  Below is a
> > > patch against 2.6.26.  Or you can just edit the original patch and
> > > change the occurrences of "fflags" to "flags".
> > > 
> > > Alan Stern
> > > 
> > > 
> > > Index: 2.6.26/drivers/usb/storage/transport.c
> > > ===================================================================
> > > --- 2.6.26.orig/drivers/usb/storage/transport.c
> > > +++ 2.6.26/drivers/usb/storage/transport.c
> > > @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
> > >  
> > >  	/* try to compute the actual residue, based on how much data
> > >  	 * was really transferred and what the device tells us */
> > > -	if (residue) {
> > > -		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
> > > +	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
> > > +
> > > +		/* Heuristically detect devices that generate bogus residues
> > > +		 * by seeing what happens with INQUIRY and READ CAPACITY
> > > +		 * commands.
> > > +		 */
> > > +		if (bcs->Status == US_BULK_STAT_OK &&
> > > +				scsi_get_resid(srb) == 0 &&
> > > +					((srb->cmnd[0] == INQUIRY &&
> > > +						transfer_length == 36) ||
> > > +					(srb->cmnd[0] == READ_CAPACITY &&
> > > +						transfer_length == 8))) {
> > > +			us->flags |= US_FL_IGNORE_RESIDUE;
> > > +
> > > +		} else {
> > >  			residue = min(residue, transfer_length);
> > >  			scsi_set_resid(srb, max(scsi_get_resid(srb),
> > >  			                                       (int) residue));
> > > 
> > > 
> > 
> > Thanks!  I've been trying to fix it manually, and it wouldn't work. 
> Trying
> > this version now.  Let's see if this fixes my problem.
> > 
> > Matt Frost
> > 
> 
> Tested under two distributions, this patch restores correct functionality
> to my hardware.  Thank you very much!
> 
> Matt
> 
Update/sqeaking-of-the-wheel:

The same problem still happens under 2.6.27-rc2, and I haven't seen this
bumped to -stable, either.  It continues to solve the problem here.

Matt

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Alan Stern]

On Fri, 8 Aug 2008, Matthew Frost wrote:

> > > > Index: 2.6.26/drivers/usb/storage/transport.c
> > > > ===================================================================
> > > > --- 2.6.26.orig/drivers/usb/storage/transport.c
> > > > +++ 2.6.26/drivers/usb/storage/transport.c
> > > > @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
> > > >  
> > > >  	/* try to compute the actual residue, based on how much data
> > > >  	 * was really transferred and what the device tells us */
> > > > -	if (residue) {
> > > > -		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
> > > > +	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
> > > > +
> > > > +		/* Heuristically detect devices that generate bogus residues
> > > > +		 * by seeing what happens with INQUIRY and READ CAPACITY
> > > > +		 * commands.
> > > > +		 */
> > > > +		if (bcs->Status == US_BULK_STAT_OK &&
> > > > +				scsi_get_resid(srb) == 0 &&
> > > > +					((srb->cmnd[0] == INQUIRY &&
> > > > +						transfer_length == 36) ||
> > > > +					(srb->cmnd[0] == READ_CAPACITY &&
> > > > +						transfer_length == 8))) {
> > > > +			us->flags |= US_FL_IGNORE_RESIDUE;
> > > > +
> > > > +		} else {
> > > >  			residue = min(residue, transfer_length);
> > > >  			scsi_set_resid(srb, max(scsi_get_resid(srb),
> > > >  			                                       (int) residue));
> > > > 
> > > > 
> > > 
> > > Thanks!  I've been trying to fix it manually, and it wouldn't work. 
> > Trying
> > > this version now.  Let's see if this fixes my problem.
> > > 
> > > Matt Frost
> > > 
> > 
> > Tested under two distributions, this patch restores correct functionality
> > to my hardware.  Thank you very much!
> > 
> > Matt
> > 
> Update/sqeaking-of-the-wheel:
> 
> The same problem still happens under 2.6.27-rc2, and I haven't seen this
> bumped to -stable, either.  It continues to solve the problem here.

The patch has not yet been pushed to mainline but it is present in Greg 
KH's USB development tree.  Presumably it will end up in the mainline 
kernel before 2.6.27-final is released.

Alan Stern

Re: [usb-storage] BUG: SCSI: usb storage SDHC card doesn't work in 2.6.27-rc1

[Matthew Frost]

--- Alan Stern <stern@rowland.harvard.edu> wrote:

> On Fri, 8 Aug 2008, Matthew Frost wrote:
> 
> > > > > Index: 2.6.26/drivers/usb/storage/transport.c
> > > > > ===================================================================
> > > > > --- 2.6.26.orig/drivers/usb/storage/transport.c
> > > > > +++ 2.6.26/drivers/usb/storage/transport.c
> > > > > @@ -1034,8 +1034,21 @@ int usb_stor_Bulk_transport(struct scsi_
> > > > >  
> > > > >  	/* try to compute the actual residue, based on how much data
> > > > >  	 * was really transferred and what the device tells us */
> > > > > -	if (residue) {
> > > > > -		if (!(us->flags & US_FL_IGNORE_RESIDUE)) {
> > > > > +	if (residue && !(us->flags & US_FL_IGNORE_RESIDUE)) {
> > > > > +
> > > > > +		/* Heuristically detect devices that generate bogus residues
> > > > > +		 * by seeing what happens with INQUIRY and READ CAPACITY
> > > > > +		 * commands.
> > > > > +		 */
> > > > > +		if (bcs->Status == US_BULK_STAT_OK &&
> > > > > +				scsi_get_resid(srb) == 0 &&
> > > > > +					((srb->cmnd[0] == INQUIRY &&
> > > > > +						transfer_length == 36) ||
> > > > > +					(srb->cmnd[0] == READ_CAPACITY &&
> > > > > +						transfer_length == 8))) {
> > > > > +			us->flags |= US_FL_IGNORE_RESIDUE;
> > > > > +
> > > > > +		} else {
> > > > >  			residue = min(residue, transfer_length);
> > > > >  			scsi_set_resid(srb, max(scsi_get_resid(srb),
> > > > >  			                                       (int) residue));
> > > > > 
> > > > > 
> > > > 
> > > > Thanks!  I've been trying to fix it manually, and it wouldn't work. 
> > > Trying
> > > > this version now.  Let's see if this fixes my problem.
> > > > 
> > > > Matt Frost
> > > > 
> > > 
> > > Tested under two distributions, this patch restores correct
> functionality
> > > to my hardware.  Thank you very much!
> > > 
> > > Matt
> > > 
> > Update/sqeaking-of-the-wheel:
> > 
> > The same problem still happens under 2.6.27-rc2, and I haven't seen this
> > bumped to -stable, either.  It continues to solve the problem here.
> 
> The patch has not yet been pushed to mainline but it is present in Greg 
> KH's USB development tree.  Presumably it will end up in the mainline 
> kernel before 2.6.27-final is released.

Thanks for the feedback!  

Matt