lkml.org 
[lkml]   [2003]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: inquiry in scsi_scan.c
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

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.
> Below some diff fragments from today.]
>
> +/*
> + * Do an INQUIRY with given length (minimum 5, maximum 255).
> + * Note: many devices react badly when given an unexpected length.
> + */
> +static int
> +scsi_do_inquiry(Scsi_Request *sreq, char *buffer, int len) {
> + Scsi_Device *sdev = sreq->sr_device;
> + unsigned char cmd[6];
> + int res;
> +
> + SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: INQUIRY (len %d) "
> + "to host %d channel %d id %d lun %d\n",
> + len, sdev->host->host_no, sdev->channel,
> + sdev->id, sdev->lun));
> +
> + memset(cmd, 0, 6);
> + cmd[0] = INQUIRY;
> + cmd[4] = len;
> + sreq->sr_cmd_len = 0;
> + sreq->sr_data_direction = SCSI_DATA_READ;
> + memset(buffer, 0, len);
> +
> + scsi_wait_req(sreq, (void *) cmd, (void *) buffer, len,
> + SCSI_TIMEOUT + 4 * HZ, 3);
> +
> + res = sreq->sr_result;
> + SCSI_LOG_SCAN_BUS(3, printk(res ? KERN_INFO "scsi scan: "
> + "INQUIRY returned code 0x%x\n" :
> + KERN_INFO "scsi scan: INQUIRY OK\n", res));
> + return res;
> +}
>
> +/*
> + * The inquiry length is inq_result[4] + 5 unless overridden.
> + */
> +static int
> +valid_inquiry_lth(int bflags, unsigned char *inq_result) {
> + int len = ((bflags & BLIST_INQUIRY_36) ? 36 :
> + (bflags & BLIST_INQUIRY_58) ? 58 :
> + inq_result[4] + 5);
> + if (len > 255)
> + len = 36; /* sanity */
> + return len;
> +}
>
> Text in scsi_probe_lun(), including two reactions to comments:
>
> static void
> scsi_probe_lun(Scsi_Request *sreq, char *inq_result, int *bflags)
> {
> Scsi_Device *sdev = sreq->sr_device; /* a bit ugly */
> unsigned char scsi_cmd[MAX_COMMAND_SIZE];
> int res, possible_inq_resp_len;
>
> /* first issue an inquiry with conservative alloc_length */
> res = scsi_do_inquiry(sreq, inq_result, 36);
>
> if (res) {
> if ((driver_byte(res) & DRIVER_SENSE) != 0 &&
> (sreq->sr_sense_buffer[2] & 0xf) == UNIT_ATTENTION &&
> sreq->sr_sense_buffer[12] == 0x28 &&
> sreq->sr_sense_buffer[13] == 0) {
> /* not-ready to ready transition - good */
> /* dpg: bogus? INQUIRY never returns UNIT_ATTENTION */
> /* aeb: seen with an USB CF card reader */
> } else
> /*
> * assume no peripheral if any other sort of error
> */
> return;
> }
>
> /*
> * Get any flags for this device.
> *
> * Some devices return only 5 bytes for an INQUIRY, but the memset
> * in scsi_do_inquiry makes sure that scsi_get_device_flags() gets
> * well-defined arguments.
> */
> *bflags = scsi_get_device_flags(&inq_result[8], &inq_result[16]);
>
> possible_inq_resp_len = valid_inquiry_lth(*bflags, inq_result);
>
> if (possible_inq_resp_len > 36) { /* do additional INQUIRY */
> res = scsi_do_inquiry(sreq, inq_result, possible_inq_resp_len);
> if (res)
> return;
>
> /*
> * The INQUIRY can change, this means the length can change.
> */
> possible_inq_resp_len = valid_inquiry_lth(*bflags, inq_result);
> }
>
> sdev->inquiry_len = possible_inq_resp_len;
>
> /*
> * Abort if the response length is less than 36?
> * No, some USB devices just produce the minimal 5-byte INQUIRY.
> *
> * ...
> */
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

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

You were using cheat codes too. You guys suck.
-- Greg to General Studebaker
User Friendly, 12/16/1997
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2005-03-22 13:32    [W:0.054 / U:4.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site