[coreboot-gerrit] Change in coreboot[master]: libpayload: usbmsc: Skip zero-length packets at end of data

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34485 )

Change subject: libpayload: usbmsc: Skip zero-length packets at end of data
......................................................................

libpayload: usbmsc: Skip zero-length packets at end of data

Some broken USB mass storage devices send another zero-length packet at
the end of the data part of a transfer if the amount of data was evenly
divisible by the packet size (which is pretty much always the case for
block reads). This packet will get interpreted as the CSW and screw up
the MSC state machine.

This patch works around this issue by retrying the CSW transfer when it
was received as exactly 0 bytes. This is the same mitigation the Linux
kernel uses and harmless for correctly behaving devices. Also tighten
validation of the CSW a little, making sure we verify the length before
we read any fields and checking the signature in addition to the tag.

Change-Id: I24f183f27b2c4f0142ba6c4b35b490c5798d0d21
Signed-off-by: Julius Werner 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34485
Reviewed-by: Patrick Georgi 
Reviewed-by: Paul Menzel 
Tested-by: build bot (Jenkins) 
---
M payloads/libpayload/drivers/usb/usbmsc.c
1 file changed, 13 insertions(+), 4 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Paul Menzel: Looks good to me, but someone else must approve

diff --git a/payloads/libpayload/drivers/usb/usbmsc.c b/payloads/libpayload/drivers/usb/usbmsc.c
index 2c7cbe5..d793e34 100644
--- a/payloads/libpayload/drivers/usb/usbmsc.c
+++ b/payloads/libpayload/drivers/usb/usbmsc.c
@@ -218,14 +218,23 @@
 static int
 get_csw (endpoint_t *ep, csw_t *csw)
 {
-	if (ep->dev->controller->bulk (ep, sizeof (csw_t), (u8 *) csw, 1) < 0) {
+	hci_t *ctrlr = ep->dev->controller;
+	int ret = ctrlr->bulk (ep, sizeof (csw_t), (u8 *) csw, 1);
+
+	/* Some broken sticks send a zero-length packet at the end of their data
+	   transfer which would show up here. Skip it to get the actual CSW. */
+	if (ret == 0)
+		ret = ctrlr->bulk (ep, sizeof (csw_t), (u8 *) csw, 1);
+
+	if (ret < 0) {
    	clear_stall (ep);
-		if (ep->dev->controller->bulk
-				(ep, sizeof (csw_t), (u8 *) csw, 1) < 0) {
+		if (ctrlr->bulk (ep, sizeof (csw_t), (u8 *) csw, 1) < 0) {
    		return reset_transport (ep->dev);
    	}
    }
-	if (csw->dCSWTag != tag) {
+	if (ret != sizeof(csw_t) || csw->dCSWTag != tag ||
+	    csw->dCSWSignature != csw_signature) {
+		usb_debug ("MSC: received malformed CSW\n");
    	return reset_transport (ep->dev);
    }
    return MSC_COMMAND_OK;