Re: [PATCH] Oops in usb-storage.c

Matthew Dharm
2001-10-17T19:15:36Z

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

This looks better... but you have a dangerous memset() in an else clause --
the request buffer pointer could be pointing to a scatter-gather list, so
you can't just memset on it.

Matt

On Wed, Oct 17, 2001 at 12:42:49PM +0200, Jan Niehusmann wrote:
> On Tue, Oct 16, 2001 at 11:24:52PM -0700, Matthew Dharm wrote:
> > Technically, as long as the system believes that the target exists (i.e.
> > you haven't unloaded your SCSI driver module), the target is required to
> > respond to an INQUIRY command. So, if you boot with the scanner on, and
> > then turn it off, you've got a problem.
>
> Ok. I looked at the SCSI-2 standard and found a possibly sensible answer
> for such an INQUIRY to a disconnected device.
> First, there is a special peripheral qualifier for disconnected
> physical devices:
>
> 001b The target is capable of supporting the specified peripheral
> device type on this logical unit; however, the physical device
> is not currently connected to this logical unit.
>
> Second, the devices is not required to give a complete answer:
>
> NOTES
> 65 The INQUIRY command is typically used by the initiator after a reset
> or power-up condition to determine the device types for system
> configuration. To minimize delays after a reset or power-up condition,
> the standard INQUIRY data should be available without incurring any
> media access delays. If the target does store some of the INQUIRY data
> on the device, it may return zeros or ASCII spaces (20h) in those fields
> until the data is available from the device.
>
> While this permission to set some fields to zero is included in the
> standard for other purposes, it makes clear that you must expect to
> get incomplete answers from an INQUIRY command.
>
> So, what about the following patch?
>
> Jan
>
>
>
> --- linux-2.4.12-ac3/drivers/usb/storage/usb.c.orig Mon Oct 1 12:15:29 2001
> +++ linux-2.4.12-ac3/drivers/usb/storage/usb.c Wed Oct 17 12:33:40 2001
> @@ -262,16 +262,28 @@
> if (data_len<36) // You lose.
> return;
>
> - memcpy(data+8, us->unusual_dev->vendorName,
> - strlen(us->unusual_dev->vendorName) > 8 ? 8 :
> - strlen(us->unusual_dev->vendorName));
> - memcpy(data+16, us->unusual_dev->productName,
> - strlen(us->unusual_dev->productName) > 16 ? 16 :
> - strlen(us->unusual_dev->productName));
> - data[32] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>12) & 0x0F);
> - data[33] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>8) & 0x0F);
> - data[34] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>4) & 0x0F);
> - data[35] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice) & 0x0F);
> + if(data[0]&0x20) { /* USB device currently not connected. Return
> + peripheral qualifier 001b ("...however, the
> + physical device is not currently connected
> + to this logical unit") and leave vendor and
> + product identification empty. ("If the target
> + does store some of the INQUIRY data on the
> + device, it may return zeros or ASCII spaces
> + (20h) in those fields until the data is
> + available from the device."). */
> + memset(data+8,0,28);
> + } else {
> + memcpy(data+8, us->unusual_dev->vendorName,
> + strlen(us->unusual_dev->vendorName) > 8 ? 8 :
> + strlen(us->unusual_dev->vendorName));
> + memcpy(data+16, us->unusual_dev->productName,
> + strlen(us->unusual_dev->productName) > 16 ? 16 :
> + strlen(us->unusual_dev->productName));
> + data[32] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>12) & 0x0F);
> + data[33] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>8) & 0x0F);
> + data[34] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>4) & 0x0F);
> + data[35] = 0x30 + ((us->pusb_dev->descriptor.bcdDevice) & 0x0F);
> + }
>
> if (us->srb->use_sg) {
> sg = (struct scatterlist *)us->srb->request_buffer;
> @@ -389,24 +401,6 @@
> break;
> }
>
> - /* Handle those devices which need us to fake their
> - * inquiry data */
> - if ((us->srb->cmnd[0] == INQUIRY) &&
> - (us->flags & US_FL_FIX_INQUIRY)) {
> - unsigned char data_ptr[36] = {
> - 0x00, 0x80, 0x02, 0x02,
> - 0x1F, 0x00, 0x00, 0x00};
> -
> - US_DEBUGP("Faking INQUIRY command\n");
> - fill_inquiry_response(us, data_ptr, 36);
> - us->srb->result = GOOD << 1;
> -
> - set_current_state(TASK_INTERRUPTIBLE);
> - us->srb->scsi_done(us->srb);
> - us->srb = NULL;
> - break;
> - }
> -
> /* lock the device pointers */
> down(&(us->dev_semaphore));
>
> @@ -422,16 +416,38 @@
> usb_stor_sense_notready,
> sizeof(usb_stor_sense_notready));
> us->srb->result = GOOD << 1;
> + } else if(us->srb->cmnd[0] == INQUIRY) {
> + unsigned char data_ptr[36] = {
> + 0x20, 0x80, 0x02, 0x02,
> + 0x1F, 0x00, 0x00, 0x00};
> + US_DEBUGP("Faking INQUIRY command for disconnected device\n");
> + fill_inquiry_response(us, data_ptr, 36);
> + us->srb->result = GOOD << 1;
> } else {
> + memset(us->srb->request_buffer, 0, us->srb->request_bufflen);
> memcpy(us->srb->sense_buffer,
> usb_stor_sense_notready,
> sizeof(usb_stor_sense_notready));
> us->srb->result = CHECK_CONDITION << 1;
> }
> } else { /* !us->pusb_dev */
> - /* we've got a command, let's do it! */
> - US_DEBUG(usb_stor_show_command(us->srb));
> - us->proto_handler(us->srb, us);
> +
> + /* Handle those devices which need us to fake
> + * their inquiry data */
> + if ((us->srb->cmnd[0] == INQUIRY) &&
> + (us->flags & US_FL_FIX_INQUIRY)) {
> + unsigned char data_ptr[36] = {
> + 0x00, 0x80, 0x02, 0x02,
> + 0x1F, 0x00, 0x00, 0x00};
> +
> + US_DEBUGP("Faking INQUIRY command\n");
> + fill_inquiry_response(us, data_ptr, 36);
> + us->srb->result = GOOD << 1;
> + } else {
> + /* we've got a command, let's do it! */
> + US_DEBUG(usb_stor_show_command(us->srb));
> + us->proto_handler(us->srb, us);
> + }
> }
>
> /* unlock the device pointers */
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

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

Dudes! May the Open Source be with you.
-- Eric S. Raymond
User Friendly, 12/3/1998

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