Bug 11125

Can you build a kernel with CONFIG_USB_STORAGE_DEBUG enabled and attach the dmesg log for the time when you plug in the iRiver T10?  Probably it needs an unusual_devs entry with the IGNORE_RESIDUE flag set; the debugging log will tell us for sure.

Created attachment 16894 [details]
debug log without the commit

This is the log with commit reverted. If you need the log with commit active, I can do it tomorrow.

Created attachment 16899 [details]
unusual_devs entry for iRiver T10

Yes, it's a bad Residue value, as I thought.  Interestingly, the value is correct for READ(10) and presumably also WRITE(10) commands -- but it's wrong for INQUIRY, READ CAPACITY, and MODE SENSE.  One has to wonder what the engineers were smoking when they designed this device.

Anyway, this patch should fix the problem.

Yes, it does.

Alan, can you please cc me on the final patch so I can ensure that
it gets into 2.6.26.x?

Thanks.

Reply-To: dougg@torque.net

bugme-daemon@bugzilla.kernel.org wrote:
> http://bugzilla.kernel.org/show_bug.cgi?id=11125
> 
>            Summary: iRiver T10 is incorrectly handled because of commit
>                     bdb2b8ca
>            Product: IO/Storage
>            Version: 2.5
>      KernelVersion: 2.6.26
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: SCSI
>         AssignedTo: linux-scsi@vger.kernel.org
>         ReportedBy: wrar@altlinux.org
> 
> 
> iRiver T10 mp3 player, which is basically an USB mass storage, isn't working
> anymore:
> 
> usb 5-1: new high speed USB device using ehci_hcd and address 5
> usb 5-1: configuration #1 chosen from 1 choice
> usb 5-1: New USB device found, idVendor=4102, idProduct=1020
> usb 5-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 5-1: Product: iriver MP3 T10
> usb 5-1: Manufacturer: iriver Limited
> usbcore: registered new interface driver libusual
> Initializing USB Mass Storage driver...
> scsi4 : SCSI emulation for USB Mass Storage devices
> usbcore: registered new interface driver usb-storage
> USB Mass Storage support registered.
> usb-storage: device found at 5
> usb-storage: waiting for device to settle before scanning
> scsi scan: INQUIRY result too short (5), using 36
> scsi 4:0:0:0: Direct-Access                                    PQ: 0 ANSI: 0
> sd 4:0:0:0: [sdc] Sector size 0 reported, assuming 512.
> sd 4:0:0:0: [sdc] 1 512-byte hardware sectors (0 MB)
> sd 4:0:0:0: [sdc] Write Protect is off
> sd 4:0:0:0: [sdc] Mode Sense: 03 00 00 00
> sd 4:0:0:0: [sdc] Assuming drive cache: write through
> sd 4:0:0:0: [sdc] Sector size 0 reported, assuming 512.
> sd 4:0:0:0: [sdc] 1 512-byte hardware sectors (0 MB)
> sd 4:0:0:0: [sdc] Write Protect is off
> sd 4:0:0:0: [sdc] Mode Sense: 03 00 00 00
> sd 4:0:0:0: [sdc] Assuming drive cache: write through
>  sdc:
> sd 4:0:0:0: [sdc] Attached SCSI disk
> sd 4:0:0:0: Attached scsi generic sg4 type 0
> usb-storage: device scan complete
> 
> 
> If I revert bdb2b8ca, it starts to work again:
> 
> usb 5-1: new high speed USB device using ehci_hcd and address 6
> usb 5-1: configuration #1 chosen from 1 choice
> scsi5 : SCSI emulation for USB Mass Storage devices
> usb 5-1: New USB device found, idVendor=4102, idProduct=1020
> usb 5-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
> usb 5-1: Product: iriver MP3 T10
> usb 5-1: Manufacturer: iriver Limited
> usb-storage: device found at 6
> usb-storage: waiting for device to settle before scanning
> scsi 5:0:0:0: Direct-Access     iriver   MP3 T10E         0.83 PQ: 0 ANSI: 2
> sd 5:0:0:0: [sdc] 1929216 512-byte hardware sectors (988 MB)
> sd 5:0:0:0: [sdc] Write Protect is off
> sd 5:0:0:0: [sdc] Mode Sense: 03 00 00 00
> sd 5:0:0:0: [sdc] Assuming drive cache: write through
> sd 5:0:0:0: [sdc] 1929216 512-byte hardware sectors (988 MB)
> sd 5:0:0:0: [sdc] Write Protect is off
> sd 5:0:0:0: [sdc] Mode Sense: 03 00 00 00
> sd 5:0:0:0: [sdc] Assuming drive cache: write through
>  sdc:
> sd 5:0:0:0: [sdc] Attached SCSI removable disk
> sd 5:0:0:0: Attached scsi generic sg4 type 0
> usb-storage: device scan complete
> 

I'm seeing exactly the same failure with a JetFlash 2GB
USB stick. Works fine with production Ubuntu 8.04 and
Fedora 9 (latest updates) but fails as shown above in
lk 2.6.26 . With my tools (sg3_utils) it just yields an
empty response to an INQUIRY command, MODE SENSE fails but
READ CAPACITY half works. When I use my tools on XP with
the same device there is no problem.

Looks like resid is wrong.


# sg_inq -vvvv /dev/sg2
open /dev/sg2 with flags=0x800
     inquiry cdb: 12 00 00 00 24 00
       duration=4 ms
     inquiry: requested 36 bytes but got 0 bytes
inquiry: got too few bytes (0)
     inquiry: failed requesting 36 byte response: res=97


 From /var/log/messages:
usb-storage: Command INQUIRY (6 bytes)
usb-storage:  12 00 00 00 24 00
usb-storage: Bulk Command S 0x43425355 T 0xe L 36 F 128 Trg 0 LUN 0 CL 6
usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
usb-storage: Status code 0; transferred 31/31
usb-storage: -- transfer complete
usb-storage: Bulk command transfer result=0
usb-storage: usb_stor_bulk_transfer_sglist: xfer 36 bytes, 1 entries
usb-storage: Status code 0; transferred 36/36
usb-storage: -- transfer complete
usb-storage: Bulk data transfer result 0x0
usb-storage: Attempting to get CSW...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code 0; transferred 13/13
usb-storage: -- transfer complete
usb-storage: Bulk status result = 0
usb-storage: Bulk Status S 0x53425355 T 0xe R 36 Stat 0x0
usb-storage: scsi cmd done, result=0x0
usb-storage: *** thread sleeping.


# sg_readcap -vvvv /dev/sg2
open /dev/sg2 with flags=0x800
     read capacity (10) cdb: 25 00 00 00 00 00 00 00 00 00
       duration=0 ms
     read capacity (10): requested 8 bytes but got 0 bytes
Read Capacity results:
    Last logical block address=3969023 (0x3c8fff), Number of blocks=3969024
    Logical block length=512 bytes
Hence:
    Device size: 2032140288 bytes, 1938.0 MiB, 2.03 GB

dmesg:
usb-storage: Command READ_CAPACITY (10 bytes)
usb-storage:  25 00 00 00 00 00 00 00 00 00
usb-storage: Bulk Command S 0x43425355 T 0xf L 8 F 128 Trg 0 LUN 0 CL 10
usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
usb-storage: Status code 0; transferred 31/31
usb-storage: -- transfer complete
usb-storage: Bulk command transfer result=0
usb-storage: usb_stor_bulk_transfer_sglist: xfer 8 bytes, 1 entries
usb-storage: Status code 0; transferred 8/8
usb-storage: -- transfer complete
usb-storage: Bulk data transfer result 0x0
usb-storage: Attempting to get CSW...
usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
usb-storage: Status code 0; transferred 13/13
usb-storage: -- transfer complete
usb-storage: Bulk status result = 0
usb-storage: Bulk Status S 0x53425355 T 0xf R 8 Stat 0x0
usb-storage: -- unexpectedly short transfer
usb-storage: scsi cmd done, result=0x0
usb-storage: *** thread sleeping



Doug Gilbert

The same behavior with iRiver T20/T30.
Here is the patch:

diff -Nupr linux-2.6.26-gentoo-old/drivers/usb/storage/unusual_devs.h linux-2.6.26-gentoo/drivers/usb/storage/unusual_devs.h
--- linux-2.6.26-gentoo-old/drivers/usb/storage/unusual_devs.h	2008-07-22 08:47:36.000000000 +0300
+++ linux-2.6.26-gentoo/drivers/usb/storage/unusual_devs.h	2008-07-22 08:38:14.000000000 +0300
@@ -1758,6 +1758,16 @@ UNUSUAL_DEV(  0x2770, 0x915d, 0x0010, 0x
 		US_SC_DEVICE, US_PR_DEVICE, NULL,
 		US_FL_FIX_CAPACITY ),
 
+/* Reported by Milinevsky Dmitry <niam.niam@gmail.com> */
+UNUSUAL_DEV(  0x4102, 0x1019, 0x0100, 0x0100,
+		"iRiver, Ltd.",
+		"iriver T30",
+		US_SC_DEVICE, US_PR_DEVICE, NULL, US_FL_IGNORE_RESIDUE ),
+UNUSUAL_DEV(  0x4102, 0x1014, 0x0100, 0x0100,
+		"iRiver, Ltd.",
+		"iriver T20",
+		US_SC_DEVICE, US_PR_DEVICE, NULL, US_FL_IGNORE_RESIDUE ),
+
 /*
  * David Härdeman <david@2gen.com>
  * The key makes the SCSI stack print confusing (but harmless) messages

I have the same problem with Apacer USB SmartCard reader. Please don't add new quirks and revert the offending patch instead as it obviously breaks many devices.

On Wed, 23 Jul 2008 bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=11125

> I have the same problem with Apacer USB SmartCard reader. Please don't add
> new
> quirks and revert the offending patch instead as it obviously breaks many
> devices.

Sorry, the offending patch cannot be removed.  Not only does it make 
the kernel properly obey the standard, it also is vitally necessary to 
fix the original problem it was created for.

The patch does not break many devices; these devices were already 
broken.  The fact that they worked at all was more or less an accident.

If you would like to submit a patch adding the appropriate quirk for 
your device to the usb-storage mailing list, it will be accepted.

Alan Stern

I wonder what device or problem does the patch fix (don't know where and how to get the actual patch from the ID "bdb2b8ca")?

Obeying the standard is not a good reason to break devices. I know that there is a lot of broken USB storage devices everywhere - but I don't think it's a good idea to list them all in a blacklist file. You'll always find another one that's broken and not listed, thus not working. Why can't we do something to workaround the bugs? 1-sector USB storage device is obviously wrong, so why don't we try the old detection method?

(In reply to comment #10)
> I wonder what device or problem does the patch fix (don't know where and how
> to
> get the actual patch from the ID "bdb2b8ca")?

https://lists.one-eyed-alien.net/pipermail/usb-storage/2008-June/003795.html

Concerning http://bugzilla.kernel.org/show_bug.cgi?id=11125

Commit bdb2b8ca fixed a real problem, but it is generating problems of
its own.  Rather than adding a bunch of new unusual_devs entries, one
at a time, I think we may be able to solve most of these new problems
in a single stroke with the following patch.  Could everyone please try
it out, with their respective new unusual_devs changes removed?

For the sake of those not familiar with the original problem, here's a 
capsule summary.  The USB Mass-Storage Bulk-only Transport allows 
devices to send invalid data back to the host.  The device indicates 
the data is invalid by putting a positive Residue value in the CSW.  It 
turns out that some devices actually do this, but the kernel was trying 
to _use_ the invalid data instead of ignoring it.

The commit mentioned above fixed that problem by zeroing out the last
Residue bytes of the device's reply.  But now it seems that some
devices erroneously set the Residue to a positive value even when the
data is entirely correct.  Commit bdb2b8ca causes the kernel to ignore
the valid data too; to fix this we have to tell usb-storage to
disregard the bogus Residue value.

Since the bad Residue shows up in the very first command, an INQUIRY, 
we ought to be able to detect these problematic devices automatically.  
That's what this patch does.

(Note: I haven't been able even to compile-test it.  So don't be too 
surprised if it doesn't work right.)

Alan Stern



Index: usb-2.6/drivers/usb/storage/transport.c
===================================================================
--- usb-2.6.orig/drivers/usb/storage/transport.c
+++ usb-2.6/drivers/usb/storage/transport.c
@@ -1033,7 +1033,15 @@ 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->fflags & US_FL_IGNORE_RESIDUE)) {
+
+		/* Heuristically detect devices that generate bogus residues
+		 * by seeing what happens with INQUIRY commands
+		 */
+		if (srb->cmnd[0] == INQUIRY && transfer_length == 36 &&
+				scsi_get_resid(srb) == 0) {
+			us->fflags |= US_FL_IGNORE_RESIDUE;
+
+		} else if (!(us->fflags & US_FL_IGNORE_RESIDUE)) {
 			residue = min(residue, transfer_length);
 			scsi_set_resid(srb, max(scsi_get_resid(srb),
 			                                       (int) residue));

Alan Stern wrote:
> Since the bad Residue shows up in the very first command, an INQUIRY, 
> we ought to be able to detect these problematic devices automatically.  
> That's what this patch does.

Neat one, Alan, I really like that.

-- 
Phil Dibowitz                             phil@ipom.com
Open Source software and tech docs        Insanity Palace of Metallica
http://www.phildev.net/                   http://www.ipom.com/

"Never write it in C if you can do it in 'awk';
 Never do it in 'awk' if 'sed' can handle it;
 Never use 'sed' when 'tr' can do the job;
 Never invoke 'tr' when 'cat' is sufficient;
 Avoid using 'cat' whenever possible" -- Taylor's Laws of Programming

Reply-To: zaitcev@redhat.com

On Wed, 23 Jul 2008 18:26:03 -0400 (EDT), Alan Stern <stern@rowland.harvard.edu> wrote:

>       if (residue) {
> -             if (!(us->fflags & US_FL_IGNORE_RESIDUE)) {
> +
> +             /* Heuristically detect devices that generate bogus residues
> +              * by seeing what happens with INQUIRY commands
> +              */
> +             if (srb->cmnd[0] == INQUIRY && transfer_length == 36 &&
> +                             scsi_get_resid(srb) == 0) {
> +                     us->fflags |= US_FL_IGNORE_RESIDUE;
> +
> +             } else if (!(us->fflags & US_FL_IGNORE_RESIDUE)) {
>                       residue = min(residue, transfer_length);

Why do you have to link this to INQUIRY? Doing the same for
any command would work, I expect.

-- Pete

On Wed, 23 Jul 2008, Pete Zaitcev wrote:

> > +           /* Heuristically detect devices that generate bogus residues
> > +            * by seeing what happens with INQUIRY commands
> > +            */

> Why do you have to link this to INQUIRY? Doing the same for
> any command would work, I expect.

For most commands it probably would.  (Although sometimes the non-zero
Residue value _isn't_ bogus.)  But if we don't do it at the very first
command, that command will fail.  The patch implicitly assumes that a
device would never send a legitimate non-zero Residue in response to a
36-byte INQUIRY.

Alan Stern

I tested the patch (had to edit fflags to flags) and it does not fix my device (maybe because it sets the IGNORE_RESIDUE flag but the IDENTIFY was already done?):
usb-storage: device found at 4
usb-storage: waiting for device to settle before scanning
scsi 2:0:0:0: Direct-Access     Generic                   6000 PQ: 0 ANSI: 0 CCS
sd 2:0:0:0: [sdb] Sector size 0 reported, assuming 512.
sd 2:0:0:0: [sdb] 1 512-byte hardware sectors (0 MB)
sd 2:0:0:0: [sdb] Write Protect is off
sd 2:0:0:0: [sdb] Mode Sense: 4b 00 00 08
sd 2:0:0:0: [sdb] Assuming drive cache: write through
sd 2:0:0:0: [sdb] Sector size 0 reported, assuming 512.
sd 2:0:0:0: [sdb] 1 512-byte hardware sectors (0 MB)
sd 2:0:0:0: [sdb] Write Protect is off
sd 2:0:0:0: [sdb] Mode Sense: 4b 00 00 08
sd 2:0:0:0: [sdb] Assuming drive cache: write through
 sdb: sdb1
 sdb: p1 exceeds device capacity
sd 2:0:0:0: [sdb] Attached SCSI removable disk
sd 2:0:0:0: Attached scsi generic sg3 type 0
usb-storage: device scan complete
attempt to access beyond end of device
sdb: rw=0, want=40, limit=1
Buffer I/O error on device sdb1, logical block 0
attempt to access beyond end of device
sdb: rw=0, want=48, limit=1
Buffer I/O error on device sdb1, logical block 1
attempt to access beyond end of device
sdb: rw=0, want=56, limit=1
Buffer I/O error on device sdb1, logical block 2
attempt to access beyond end of device
sdb: rw=0, want=64, limit=1
Buffer I/O error on device sdb1, logical block 3
attempt to access beyond end of device
sdb: rw=0, want=40, limit=1
Buffer I/O error on device sdb1, logical block 0
attempt to access beyond end of device
sdb: rw=0, want=40, limit=1
Buffer I/O error on device sdb1, logical block 0
attempt to access beyond end of device
sdb: rw=0, want=48, limit=1
Buffer I/O error on device sdb1, logical block 1
attempt to access beyond end of device
sdb: rw=0, want=56, limit=1
Buffer I/O error on device sdb1, logical block 2
attempt to access beyond end of device
sdb: rw=0, want=64, limit=1
Buffer I/O error on device sdb1, logical block 3
attempt to access beyond end of device
sdb: rw=0, want=40, limit=1
Buffer I/O error on device sdb1, logical block 0

Can you do the same thing with CONFIG_USB_STORAGE_DEBUG enabled, and attach the corresponding dmesg log to this bug report?

(In reply to comment #12)
> Index: usb-2.6/drivers/usb/storage/transport.c
Seems to work here.

(In reply to comment #16)
> I tested the patch (had to edit fflags to flags)
Why did you do that?

> (In reply to comment #16)
> > I tested the patch (had to edit fflags to flags)
> Why did you do that?
> 

Because the patch didn't apply without that. Maybe I'm doing something wrong (applying the patch to 2.6.26 kernel).

Created attachment 17011 [details]
debug log with Apacer USB SD card reader

Created attachment 17012 [details]
Residue heuristic using INQUIRY and READ CAPACITY

It turns out that in your case the bad Residue is attached to a READ CAPACITY command and not to the initial INQUIRY command.  This is an updated version of the patch which should detect either error.  (You'll still have to edit it to change fflags to flags; this patch is based on a development tree instead of vanilla 2.6.26.)

Created attachment 17013 [details]
Residue heuristic using INQUIRY and READ CAPACITY (corrected)

Sorry -- this patch now has proper parentheses and no syntax errors.

Reply-To: dougg@torque.net

Alan Stern wrote:
> Concerning http://bugzilla.kernel.org/show_bug.cgi?id=11125
> 
> Commit bdb2b8ca fixed a real problem, but it is generating problems of
> its own.  Rather than adding a bunch of new unusual_devs entries, one
> at a time, I think we may be able to solve most of these new problems
> in a single stroke with the following patch.  Could everyone please try
> it out, with their respective new unusual_devs changes removed?
> 
> For the sake of those not familiar with the original problem, here's a 
> capsule summary.  The USB Mass-Storage Bulk-only Transport allows 
> devices to send invalid data back to the host.  The device indicates 
> the data is invalid by putting a positive Residue value in the CSW.  It 
> turns out that some devices actually do this, but the kernel was trying 
> to _use_ the invalid data instead of ignoring it.
> 
> The commit mentioned above fixed that problem by zeroing out the last
> Residue bytes of the device's reply.  But now it seems that some
> devices erroneously set the Residue to a positive value even when the
> data is entirely correct.  Commit bdb2b8ca causes the kernel to ignore
> the valid data too; to fix this we have to tell usb-storage to
> disregard the bogus Residue value.
> 
> Since the bad Residue shows up in the very first command, an INQUIRY, 
> we ought to be able to detect these problematic devices automatically.  
> That's what this patch does.
> 
> (Note: I haven't been able even to compile-test it.  So don't be too 
> surprised if it doesn't work right.)
> 
> Alan Stern
> 
> 
> 
> Index: usb-2.6/drivers/usb/storage/transport.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/storage/transport.c
> +++ usb-2.6/drivers/usb/storage/transport.c
> @@ -1033,7 +1033,15 @@ 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->fflags & US_FL_IGNORE_RESIDUE)) {
> +
> +             /* Heuristically detect devices that generate bogus residues
> +              * by seeing what happens with INQUIRY commands
> +              */
> +             if (srb->cmnd[0] == INQUIRY && transfer_length == 36 &&
> +                             scsi_get_resid(srb) == 0) {
> +                     us->fflags |= US_FL_IGNORE_RESIDUE;
> +
> +             } else if (!(us->fflags & US_FL_IGNORE_RESIDUE)) {
>                       residue = min(residue, transfer_length);
>                       scsi_set_resid(srb, max(scsi_get_resid(srb),
>                                                              (int) residue));
> 

Alan,
Thanks for fixing this.

I would like to point out that the response to a SCSI
INQUIRY command should not be complete "crap" or a
null response. Obviously the transport layer and lower
can fail but that should be handled in different ways.

Taking the latest SPC-4 draft:
http://www.t10.org/ftp/t10/drafts/spc4/spc4r15.pdf
section 6.4.1 on page 240 there is a paragraph that
states:
"The INQUIRY data should be returned even though the device server
is not ready for other commands. The standard INQUIRY data should
be available without incurring any media access delays. If the
device server does store some of the standard INQUIRY data or VPD
data on the media, it may return ASCII spaces (20h) in ASCII
fields and zeros in other fields until data is available from the
media."

The INQUIRY response is special in this respect.

My interpretation of the above is that the USB mass storage
driver needs to look at what it is sending "up" as an INQUIRY
response and sanitize it if necessary. If the USB mass storage
device is defective then either work round the defect or report
to the SCSI subsystem that there is no (SCSI) device there.

Doug Gilbert

On Mon, 28 Jul 2008, Douglas Gilbert wrote:

> My interpretation of the above is that the USB mass storage
> driver needs to look at what it is sending "up" as an INQUIRY
> response and sanitize it if necessary. If the USB mass storage
> device is defective then either work round the defect or report
> to the SCSI subsystem that there is no (SCSI) device there.

In this case the INQUIRY data are entirely correct.  The only problem 
is that the Residue value is nonzero when it should really be 0.  This 
is a common sort of problem; devices that can't respond to INQUIRY 
won't work with Windows, but Windows ignores the Residue.

The patch I posted does exactly what you suggest -- it works around the 
defect by ignoring the device's Residue and returning a Residue of 0.

Alan Stern

Thank you very much for the work. My Apacer SD card reader works fine with the patch.

The patch has been submitted for 2.6.27.  When it has been accepted I will also submit it for 2.6.26-stable.

*** Bug 11183 has been marked as a duplicate of this bug. ***

Accepted into mainline: commit 59f4ff2ecff4cef36378928cec891785b402e80c