Re: inquiry in scsi_scan.c

[Matthew Dharm <mdharm-kernel@one-eyed-alien.net>]

[-- Attachment #1: Type: text/plain, Size: 5919 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]