Hi all,
On 08/15/2010 02:22 AM, Matthew Dharm wrote:
On Sat, Aug 14, 2010 at 06:14:09PM -0500, James Bottomley wrote:
On Sat, 2010-08-14 at 15:36 -0700, Matthew Dharm wrote:
That said, James has not yet addressed my point about allowing userspace
tools to attempt the command, if they believe they can generate it.
Well, that's because it didn't really make sense: the same would apply
to INQUIRY which you already filter. Plus if there is a way to send the
commands that doesn't crash the device, we should likely be using it.
Although I don't think so: READ DISC INFORMATION has no parameters at
all and the only parameter for READ CAPACITY(16) is the partial medium
indicator which is pretty much never used (So I'd be really surprised if
any USB devices understood it).
There's no point in SCSI having a flag which says "I don't support this
optional command" because we have to expect any device to say "I don't
support this command" with the correct return code ... so you might as
well just return the correct code and not add the flag.
Well... if it's really completely optional.... I suppose filtering it in
USB would be reasonable. I still don't like it, but I recognize that I'm
not going to change your mind, and something needs to be done.
INQUIRY was filtered only after months and months of work to identify the
most "friendly" INQUIRY that could be sent to the device, and filtering in
usb-storage was added as a very very last resort.
So far I (the author of the original patches) have been following this from
the sidelines. When it comes to the troubles with READ DISC INFORMATION, then
indeed simply filtering them out the usb mass storage level and return a sense
code that it is not supported will work.
Actually this is what my first version of the patch did ...
I must add a note here though, that the sending of READ DISC INFORMATION to
begin with is a bit of a dubious hack and that that very popular OS from Redmond
does not do that (according to the usb traces I've analyzed). The sending
of the READ DISC INFORMATION command was added relatively recent (it is the
second last commit or so on the cdrom driver), to work around bugs in some
firmware.
The case of the READ CAPACITY(16) problem is different and I'm not sure that
can be fixed easily at the usb layer. The problem here is as follows:
1) cheap mp3/mp4 player with internal storage and a sdcard slot
2) sdcard slot is empty (how it is shipped)
3) When probing the 2nd lun of the device (the sdcard slot) the firmware
tries to answer to READ CAPACITY(10) with a capacity of 0 (or so I believe,
the menu on the player lists the sdcard slot as having a 0 bytes card in
there)
4) The firmware authors actually got READ CAPACITY right in as far as that
they return "size_in_sectors - 1", so that makes them return "0 - 1" aka
0xffffffff
5) The return of 0xffffffff gets seen by Linux as a capacity of 0x100000000
sectors and then it issues a READ CAPACITY(16) command -> boom.
My fix here consists of 2 things, both triggered by the no_read_capacity16
flag:
1) Don't issue READ CAPACITY(16) (this could be filtered out in the usb
layer); and
2) When READ CAPACITY(10) returns 0xffffffff and the no_read_capacity16
flag is set, set capacity to 0 rather then 0x100000000.
2) Cannot be done in the usb layer! I know the right thing to do would
be to fake a no medium sense code, but that is asking for a lot more
complicated code then simply rejecting a command. The current fix with
setting capacity to 0 works nicely. And note that this is something that
userspace needs to be able to deal with already anyways as some other block
device drivers (*cough* cciss *cough*) sometimes do this too.
If the fix is changed to simply reject READ CAPACITY(16), the second LUN
will get seen as having a capacity of 0x100000000 bytes, which is wrong.
Moreover more commands (for reading the partition table for example) will
then get issues to the device and I find it likely it won't like those a
whole lot either.
Regards,
Hans
--
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
