g: upgrade - improve verification of the first upgrade message

When an upgrade transfer starts, the target expects to receive a 12
byte transfer start message from the host, carrying an empty payload
of all zeros.

The target and host can be out of sync for whatever reason, so when
the target is expecting a transfer start message, the host can be
sending a chunk of a code block instead. In this case the target pulls
just 12 bytes off the receive queue, leaving the rest of the chunk
there, which even more complicates error recovery.

This patch makes sure that when expecting the upgrade transfer start
message the target extracts all receive data from the queue, no matter
how many bytes have been received, and then verifies that the size
matches the expected size, and that the payload is all zeros, to
confirm that the message is indeed the transfer start message.

BRANCH=none
BUG=chrome-os-partner:52586
TEST=cr50 firmware upgrade still works fine on B1

Change-Id: If5ec86d0385f97f0f361635b3903cea3a962b707
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/341618
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

1 files changed, 35 insertions, 6 deletions

diff --git a/chip/g/usb_upgrade.c b/chip/g/usb_upgrade.c
index 634556cffb..182c40d7d2 100644
--- a/chip/g/usb_upgrade.c
+++ b/chip/g/usb_upgrade.c
@@ -92,6 +92,40 @@ static uint32_t block_size;
 static uint32_t block_index;
 
 /*
+ * Verify that the contens of the USB rx queue is a valid transfer start
+ * message from host, and if so - save its contents in the passed in
+ * update_pdu_header structure.
+ */
+static int valid_transfer_start(struct consumer const *consumer, size_t count,
+				struct update_pdu_header *pupdu)
+{
+	int i;
+
+	/*
+	 * Let's just make sure we drain the queue no matter what the contents
+	 * are. This way they won't be in the way during next callback, even
+	 * if these contents are not what's expected.
+	 */
+	i = count;
+	while (i > 0) {
+		QUEUE_REMOVE_UNITS(consumer->queue, pupdu,
+				   MIN(i, sizeof(*pupdu)));
+		i -= sizeof(*pupdu);
+	}
+
+	if (count != sizeof(struct update_pdu_header)) {
+		CPRINTS("FW update: wrong first block, size %d", count);
+		return 0;
+	}
+
+	/* In the first block the payload (updu.cmd) must be all zeros. */
+	for (i = 0; i < sizeof(pupdu->cmd); i++)
+		if (((uint8_t *)&pupdu->cmd)[i])
+			return 0;
+	return 1;
+}
+
+/*
  * When was last time a USB callback was called, in microseconds, free running
  * timer.
  */
@@ -149,13 +183,8 @@ static void upgrade_out_handler(struct consumer const *consumer, size_t count)
 			uint32_t version;
 		} startup_resp;
 
-		/* This better be the first block, of zero size. */
-		if (count != sizeof(struct update_pdu_header)) {
-			CPRINTS("FW update: wrong first block size %d",
-				count);
+		if (!valid_transfer_start(consumer, count, &updu))
 			return;
-		}
-		QUEUE_REMOVE_UNITS(consumer->queue, &updu, count);
 
 		CPRINTS("FW update: starting...");