Re: inquiry in scsi_scan.c

[Douglas Gilbert <dougg@torque.net>]

Matthew Dharm wrote:
> Actually, 5 isn't minimal... it's sub-minimal.  That's an error in the
> INQUIRY data.  The minimum (by spec) is 36 bytes.
> 
> There should probably be a sanity check to never ask for INQUIRY less than
> 36 bytes.  I thought there used to be such a thing....

Matt,
The scan code does a 36 byte INQUIRY first and then does
a second INQUIRY if the response indicates more (than 36
bytes) is available. In this case _less_ than 36 bytes
was supplied. So either the scsi scan code or usb-storage **
needs to make some pro forma vendor, model and rev strings.

We found previously that doing a longer INQUIRY (254
or 255 bytes) locked up some [arguably broken] usb devices.
Hence we switched to the twin INQUIRY strategy. Evidentally
FreeBSD does the same thing.

Throwing away such broken devices is not an option I guess.
What does that device look like under Windows?


** Assuming the usb device in question is using usb-storage,
shouldn't it make sure at least 36 (valid) bytes of response
data is supplied by an INQUIRY? Andrew Morton had to do this
(perhaps at a lower level in the usb stack) to make some device
he had play with the scsi subsystem.

Doug Gilbert

> On Sat, Jan 04, 2003 at 01:21:11AM +0100, Andries.Brouwer@cwi.nl wrote:
> 
>>Got a new USB device and noticed some scsi silliness while playing with it.
>>
>>A bug in scsi_scan.c is
>>
>>        sdev->inquiry = kmalloc(sdev->inquiry_len, GFP_ATOMIC);
>>        memcpy(sdev->inquiry, inq_result, sdev->inquiry_len);
>>        sdev->vendor = (char *) (sdev->inquiry + 8);
>>        sdev->model = (char *) (sdev->inquiry + 16);
>>        sdev->rev = (char *) (sdev->inquiry + 32);
>>
>>since it happens that inquiry_len is short (in my case it is 5)
>>and the vendor/model/rev pointers are wild.
>>Catting /proc/scsi/scsi now yields random garbage.
>>
>>I made a patch but hesitated between a small patch and a larger one.
>>Why do we have this malloced inquiry field? As far as I can see
>>nobody uses it. And the comment in scsi_add_lun() advises us
>>not to save the inquiry, precisely what we did until recently.
>>So, should this change from 2.5.11 be reverted?
>>
>>Andries
>>
>>
>>[My present scsi_scan.c differes quite a lot from a stock one.
>>Already fixed the scsi_check_id_size() some time ago.