On Tue, 2009-08-11 at 10:53 -0400, Alan Stern wrote:
> On Tue, 11 Aug 2009, Boaz Harrosh wrote:
>
> > This is certainly a bug. Otherwise I would get all my pages 4 bytes short
> > and wonder why.
> >
> > I wish the bug would explain that stupid USB device Martin was fixing.
> > "I die if evpd page=0 is read" is a very brain dead thing. But there
> > is no overflow in current code, only underflow.
> >
> > If you are at it could you please fix all the bugs in this code: ;-)
>
> The USB problem shouldn't affect anything thanks to Martin's other
> changes (sd won't read VPD for devices with scsi_level <= SCSI_2). So
> how does this revised patch look?
>
> Alan Stern
>
>
> Index: usb-2.6/drivers/scsi/scsi.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/scsi.c
> +++ usb-2.6/drivers/scsi/scsi.c
> @@ -969,7 +969,7 @@ EXPORT_SYMBOL(scsi_track_queue_full);
> * @sdev: The device to ask
> * @buffer: Where to put the result
> * @page: Which Vital Product Data to return
> - * @len: The length of the buffer
> + * @len: The length of the data (= buffer length - 4)
Really, no. The former is the correct (and universally used
definition). This one you propose is asking for confusion and misuse.
> *
> * This is an internal helper function. You probably want to use
> * scsi_get_vpd_page instead.
> @@ -980,7 +980,10 @@ static int scsi_vpd_inquiry(struct scsi_
> u8 page, unsigned len)
> {
> int result;
> - unsigned char cmd[16];
> + int resid;
> + unsigned char cmd[6];
> +
> + len += 4; /* Include room for the header bytes */
>
> cmd[0] = INQUIRY;
> cmd[1] = 1; /* EVPD */
> @@ -989,17 +992,19 @@ static int scsi_vpd_inquiry(struct scsi_
> cmd[4] = len & 0xff;
> cmd[5] = 0; /* Control byte */
>
> + buffer[1] = ~page;
This is pointless and dangerous: Some architectures will invalidate
caches for DMA not flush them, so it might not do what you think it
does.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
