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

Matthew Dharm (mdharm-kernel@one-eyed-alien.net)
Wed, 17 Oct 2001 12:15:40 -0700


--EVF5PPMfhYS0aIcm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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:
>=20
> 001b The target is capable of supporting the specified peripheral
> device type on this logical unit; however, the physical device=20
> is not currently connected to this logical unit. =20
>=20
> Second, the devices is not required to give a complete answer:
>=20
> 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.
>=20
> 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.
>=20
> So, what about the following patch?
>=20
> Jan
>=20
>=20
> =09
> --- linux-2.4.12-ac3/drivers/usb/storage/usb.c.orig Mon Oct 1 12:15:29 2=
001
> +++ 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;
> =20
> - memcpy(data+8, us->unusual_dev->vendorName,=20
> - strlen(us->unusual_dev->vendorName) > 8 ? 8 :
> - strlen(us->unusual_dev->vendorName));
> - memcpy(data+16, us->unusual_dev->productName,=20
> - strlen(us->unusual_dev->productName) > 16 ? 16 :
> - strlen(us->unusual_dev->productName));
> - data[32] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>12) & 0x0F);
> - data[33] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>8) & 0x0F);
> - data[34] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>4) & 0x0F);
> - data[35] =3D 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=20
> + (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,=20
> + strlen(us->unusual_dev->vendorName) > 8 ? 8 :
> + strlen(us->unusual_dev->vendorName));
> + memcpy(data+16, us->unusual_dev->productName,=20
> + strlen(us->unusual_dev->productName) > 16 ? 16 :
> + strlen(us->unusual_dev->productName));
> + data[32] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>12) & 0x0F);
> + data[33] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>8) & 0x0F);
> + data[34] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice>>4) & 0x0F);
> + data[35] =3D 0x30 + ((us->pusb_dev->descriptor.bcdDevice) & 0x0F);
> + }
> =20
> if (us->srb->use_sg) {
> sg =3D (struct scatterlist *)us->srb->request_buffer;
> @@ -389,24 +401,6 @@
> break;
> }
> =20
> - /* Handle those devices which need us to fake their
> - * inquiry data */
> - if ((us->srb->cmnd[0] =3D=3D INQUIRY) &&
> - (us->flags & US_FL_FIX_INQUIRY)) {
> - unsigned char data_ptr[36] =3D {
> - 0x00, 0x80, 0x02, 0x02,
> - 0x1F, 0x00, 0x00, 0x00};
> -
> - US_DEBUGP("Faking INQUIRY command\n");
> - fill_inquiry_response(us, data_ptr, 36);
> - us->srb->result =3D GOOD << 1;
> -
> - set_current_state(TASK_INTERRUPTIBLE);
> - us->srb->scsi_done(us->srb);
> - us->srb =3D NULL;
> - break;
> - }
> -
> /* lock the device pointers */
> down(&(us->dev_semaphore));
> =20
> @@ -422,16 +416,38 @@
> usb_stor_sense_notready,=20
> sizeof(usb_stor_sense_notready));
> us->srb->result =3D GOOD << 1;
> + } else if(us->srb->cmnd[0] =3D=3D INQUIRY) {
> + unsigned char data_ptr[36] =3D {
> + 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 =3D GOOD << 1;
> } else {
> + memset(us->srb->request_buffer, 0, us->srb->request_bufflen);
> memcpy(us->srb->sense_buffer,=20
> usb_stor_sense_notready,=20
> sizeof(usb_stor_sense_notready));
> us->srb->result =3D 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=20
> + * their inquiry data */
> + if ((us->srb->cmnd[0] =3D=3D INQUIRY) &&
> + (us->flags & US_FL_FIX_INQUIRY)) {
> + unsigned char data_ptr[36] =3D {
> + 0x00, 0x80, 0x02, 0x02,
> + 0x1F, 0x00, 0x00, 0x00};
> +
> + US_DEBUGP("Faking INQUIRY command\n");
> + fill_inquiry_response(us, data_ptr, 36);
> + us->srb->result =3D 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);
> + }
> }
> =20
> /* 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/

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

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

--EVF5PPMfhYS0aIcm
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE7zdjcz64nssGU+ykRAlKrAKCF1MZIoHnXZEf6i5fL3f3jBPV4YgCeOTGP
EcEw9ND1rjb/bfubwEmPg7g=
=wsZ5
-----END PGP SIGNATURE-----

--EVF5PPMfhYS0aIcm--
-
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/