summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulius Werner <jwerner@chromium.org>2014-05-02 14:51:03 -0700
committerMarc Jones <marc.jones@se-eng.com>2014-12-30 19:00:38 +0100
commit7124788b335b88c913b04802ddd84db0c438ccc0 (patch)
treec80eb00b34ded381219e01a4543c6ccd3de1dd12
parent87accccdc2dda421dee967badbdbc701fea0753e (diff)
downloadcoreboot-7124788b335b88c913b04802ddd84db0c438ccc0.tar.xz
libpayload: usbmsc: Set correct allocation length for REQUEST SENSE
So I was debugging this faulty USB SD card reader that would just fail it's REQUEST SENSE response for some reason (sending the CSW immediately without the data), cursing those damn device vendors for building non-compliant crap like I always do... when I noticed that we do not actually set the Allocation Length field in our REQUEST SENSE command block at all! We set a length in the CBW, but the SCSI command still has its own length field and the SCSI spec specifically says that the device has to return the exact amount of bytes listed there (even if it's 0). I don't know what's more suprising: that we had such a blatant bug in this stack for so long, or that this card reader is really the first device to actually be spec compliant in that regard. This patch fixes the bug and changes the command block structures to be a little easier to read (why that field was called 'lun' before is beyond me... LUN is a transport level thing and should never appear in the command block at all, for any command). It also fixes a memcpy() in wrap_cbw() to avoid a read buffer overflow that might expose stack frame data to the device. BRANCH=rambi?,nyan BUG=chrome-os-partner:28437 TEST=The card reader works now (for it's first LUN at least). Original-Change-Id: I86fdcae2ea4d2e2939e3676d31d8b6a4e797873b Original-Signed-off-by: Julius Werner <jwerner@chromium.org> Original-Reviewed-on: https://chromium-review.googlesource.com/198100 Original-Reviewed-by: Aaron Durbin <adurbin@chromium.org> (cherry picked from commit 88943d9715994a14c50e74170f2453cceca0983b) Signed-off-by: Marc Jones <marc.jones@se-eng.com> Change-Id: I3097c223248c07c866a33d4ab8f3db1a7082a815 Reviewed-on: http://review.coreboot.org/7903 Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Tested-by: build bot (Jenkins) Reviewed-by: Edward O'Callaghan <eocallaghan@alterapraxis.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
-rw-r--r--payloads/libpayload/drivers/usb/usbmsc.c22
1 files changed, 15 insertions, 7 deletions
diff --git a/payloads/libpayload/drivers/usb/usbmsc.c b/payloads/libpayload/drivers/usb/usbmsc.c
index 178f982e07..3b5206ef85 100644
--- a/payloads/libpayload/drivers/usb/usbmsc.c
+++ b/payloads/libpayload/drivers/usb/usbmsc.c
@@ -205,7 +205,7 @@ wrap_cbw (cbw_t *cbw, int datalen, cbw_direction dir, const u8 *cmd,
cbw->dCBWDataTransferLength = datalen;
cbw->bmCBWFlags = dir;
- memcpy (cbw->CBWCB, cmd, sizeof (cbw->CBWCB));
+ memcpy (cbw->CBWCB, cmd, cmdlen);
cbw->bCBWCBLength = cmdlen;
}
@@ -290,7 +290,7 @@ typedef struct {
unsigned int block; //2-5
unsigned char res2; //6
unsigned short numblocks; //7-8
- unsigned char res3; //9 - the block is 10 bytes long
+ unsigned char control; //9 - the block is 10 bytes long
} __attribute__ ((packed)) cmdblock_t;
typedef struct {
@@ -298,8 +298,14 @@ typedef struct {
unsigned char res1; //1
unsigned char res2; //2
unsigned char res3; //3
- unsigned char lun; //4
- unsigned char res4; //5
+ union { //4
+ struct {
+ unsigned long start:1; // for START STOP UNIT
+ unsigned long:7;
+ };
+ unsigned char length; // for REQUEST SENSE
+ };
+ unsigned char control; //5
} __attribute__ ((packed)) cmdblock6_t;
/**
@@ -409,9 +415,10 @@ request_sense (usbdev_t *dev)
cmdblock6_t cb;
memset (&cb, 0, sizeof (cb));
cb.command = 0x3;
+ cb.length = sizeof (buf);
return execute_command (dev, cbw_direction_data_in, (u8 *) &cb,
- sizeof (cb), buf, 19, 1);
+ sizeof (cb), buf, sizeof (buf), 1);
}
static int request_sense_no_media (usbdev_t *dev)
@@ -421,9 +428,10 @@ static int request_sense_no_media (usbdev_t *dev)
cmdblock6_t cb;
memset (&cb, 0, sizeof (cb));
cb.command = 0x3;
+ cb.length = sizeof (buf);
ret = execute_command (dev, cbw_direction_data_in, (u8 *) &cb,
- sizeof (cb), buf, 19, 1);
+ sizeof (cb), buf, sizeof (buf), 1);
if (ret)
return ret;
@@ -458,7 +466,7 @@ spin_up (usbdev_t *dev)
cmdblock6_t cb;
memset (&cb, 0, sizeof (cb));
cb.command = 0x1b;
- cb.lun = 1;
+ cb.start = 1;
return execute_command (dev, cbw_direction_data_out, (u8 *) &cb,
sizeof (cb), 0, 0, 0);
}