Re: [PATCH] SCSI core: always store >= 36 bytes of INQUIRY data

[Douglas Gilbert <dougg@torque.net>]

Patrick Mansfield wrote:
> On Thu, Oct 26, 2006 at 05:11:27PM -0400, Alan Stern wrote:
>> This patch (as810) sets the length of the INQUIRY data to a minimum of
>> 36 bytes, even if the device claims that not all of them are valid.
>> Using the data sent by the device is better than allocating a short
>> buffer and then reading beyond the end of it, which is what we do now.
>>
>> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
>>
>> ---
>>
>> Index: usb-2.6/drivers/scsi/scsi_scan.c
>> ===================================================================
>> --- usb-2.6.orig/drivers/scsi/scsi_scan.c
>> +++ usb-2.6/drivers/scsi/scsi_scan.c
>> @@ -575,6 +575,19 @@ static int scsi_probe_lun(struct scsi_de
>>  	 * short INQUIRY), an abort here prevents any further use of the
>>  	 * device, including spin up.
>>  	 *
>> +	 * On the whole, the best approach seems to be to assume the first
>> +	 * 36 bytes are valid no matter what the device says.  That's
> 
> The comment is confusing, as it implies the device will modify data past
> the indicated length, but well behaved devices should not do that, and
> with your patch should point to zero filled data.
> 
> Just comment on what its avoiding or such like:
> 
> 	Modify short inquiry_len values so we don't later point at random
> 	values. Devices returning an incorrect value in the INQUIRY
> 	additional length field will point at potentially valid data for
> 	Vendor, Product and Revsion, while conforming devices will point
> 	to zero filled data.
> 
> But definitely better to use possibly valid data for broken devices or
> NUL for well behaved devices rather than garbage values.
> 
>> +	 * better than copying < 36 bytes to the inquiry-result buffer
>> +	 * and displaying garbage for the Vendor, Product, and Revision
>> +	 * strings.
>> +	 */
>> +	if (sdev->inquiry_len < 36) {
>> +		printk(KERN_INFO "scsi scan: INQUIRY result too short (%d),"
>> +				" using 36\n", sdev->inquiry_len);
>> +		sdev->inquiry_len = 36;
>> +	}
>> +
>> +	/*

Some more information on this subject: LLDs (including
pseudo ones like usb-storage) really should set the
Scsi_Cmnd::resid field to show, in the case of an
INQUIRY, when 36 bytes were requested, less bytes
were returned.

That OS from Redmond may have a hand here as well.
Directly attached ATA disks are found in SCSI scans and
respond to SCSI INQUIRY commands with plausible vendor,
model and revision strings but the additional length
field (byte 4 in the response) is set to 0. So code
could use that as a hint to stop sending further SCSI
commands and start sending ATA commands. ATA disks
behind USB and IEEE 1394 don't appear in SCSI scans
but do appear as "Physical Drives" and do respond
to SCSI INQUIRY commands properly. Then there are
ATA disks behind a SATL and they respond to SCSI
INQUIRY commands (and most others) properly; the
easiest way to spot them is that the vendor is "ATA     ".

Doug Gilbert