No. No no no.
The ISD200 code was written by the ISD200 developers. I really don't want
to go mucking about changing what commands actually get send to the ISD200
parts. We have no idea if the will reliably accept a 36-byte INQUIRY.
Just because it happens to work for a couple of people doesn't mean it
works in the general case. Without guidance from In-System, this is a bad
idea.
The way to deal with this is to do this via bounce buffering. The two
commands affected (INQUIRY and MODE_SENSE) are low-performance items.
Discarding data from the end of them is also perfectly legal per spec.
Also, the entire idea of a negative resid makes my head hurt. That sort of
change is in the category of "likely to break something else" which only
expects positive resid values.
Matt
On Thu, Jan 31, 2008 at 09:37:13PM +0200, Boaz Harrosh wrote:
> Greg KH <greg@xxxxxxxxx> rote:
> > As this is a regression and hits 2.6.24, can you send the final version
> > of this patch to the stable@xxxxxxxxxx address so we can get it into the
> > 2.6.24.y tree?
> >
> > thanks,
> >
> > greg k-h
>
> Mark - This is for you on top of vanila v2.6.24 kernel from Linus.
>
> ---
>
> scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would
> volunteer 96 bytes of INQUIRY. This caused an overflow condition in
> protocol.c usb_stor_access_xfer_buf(). So first fix is to
> usb_stor_access_xfer_buf() to properly handle overflow/underflow conditions.
> Then usb_stor_set_xfer_buf() should report this condition as a negative
> resid. Should we also set cmnd->status in the overflow condition?
>
> Then also isd200.c is fixed to only return the type of INQUIRY && SENSE
> the upper layer asked for.
>
> Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> ---
> drivers/usb/storage/isd200.c | 7 +++++--
> drivers/usb/storage/protocol.c | 13 +++++++++----
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
> index 49ba6c0..8186e93 100644
> --- a/drivers/usb/storage/isd200.c
> +++ b/drivers/usb/storage/isd200.c
> @@ -1238,6 +1238,7 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb, struct us_data *us,
> unsigned long lba;
> unsigned long blockCount;
> unsigned char senseData[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
> + unsigned xfer_len;
>
> memset(ataCdb, 0, sizeof(union ata_cdb));
>
> @@ -1247,8 +1248,9 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb, struct us_data *us,
> US_DEBUGP(" ATA OUT - INQUIRY\n");
>
> /* copy InquiryData */
> + xfer_len = min(sizeof(info->InquiryData), scsi_bufflen(srb));
> usb_stor_set_xfer_buf((unsigned char *) &info->InquiryData,
> - sizeof(info->InquiryData), srb);
> + xfer_len, srb);
> srb->result = SAM_STAT_GOOD;
> sendToTransport = 0;
> break;
> @@ -1257,7 +1259,8 @@ static int isd200_scsi_to_ata(struct scsi_cmnd *srb, struct us_data *us,
> US_DEBUGP(" ATA OUT - SCSIOP_MODE_SENSE\n");
>
> /* Initialize the return buffer */
> - usb_stor_set_xfer_buf(senseData, sizeof(senseData), srb);
> + xfer_len = min(sizeof(senseData), scsi_bufflen(srb));
> + usb_stor_set_xfer_buf(senseData, xfer_len, srb);
>
> if (info->DeviceFlags & DF_MEDIA_STATUS_ENABLED)
> {
> diff --git a/drivers/usb/storage/protocol.c b/drivers/usb/storage/protocol.c
> index 889622b..038a284 100644
> --- a/drivers/usb/storage/protocol.c
> +++ b/drivers/usb/storage/protocol.c
> @@ -194,7 +194,7 @@ unsigned int usb_stor_access_xfer_buf(unsigned char *buffer,
> * and the starting offset within the page, and update
> * the *offset and *index values for the next loop. */
> cnt = 0;
> - while (cnt < buflen) {
> + while (cnt < buflen && sg) {
> struct page *page = sg_page(sg) +
> ((sg->offset + *offset) >> PAGE_SHIFT);
> unsigned int poff =
> @@ -248,9 +248,14 @@ void usb_stor_set_xfer_buf(unsigned char *buffer,
> {
> unsigned int offset = 0;
> struct scatterlist *sg = NULL;
> + unsigned count;
>
> - usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> + count = usb_stor_access_xfer_buf(buffer, buflen, srb, &sg, &offset,
> TO_XFER_BUF);
> - if (buflen < srb->request_bufflen)
> - srb->resid = srb->request_bufflen - buflen;
> +
> + /* Check for overflow */
> + if (buflen > scsi_bufflen(srb))
> + count = buflen;
> +
> + scsi_set_resid(srb, scsi_bufflen(srb) - count);
> }
> --
> 1.5.3.3
>
--
Matthew Dharm Home: mdharm-usb@xxxxxxxxxxxxxxxxxx
Maintainer, Linux USB Mass Storage Driver
M: No, Windows doesn't have any nag screens.
C: Then what are those blue and white screens I get every day?
-- Mike and Cobb
User Friendly, 1/4/1999
Attachment:
pgpKPzepFgzFr.pgp
Description: PGP signature
