From a75f7c8680afd3edbb967dbdf664e421904e6c76 Mon Sep 17 00:00:00 2001 From: Vadim Bendebury Date: Fri, 2 Jun 2017 16:05:36 -0700 Subject: [PATCH] cr50: usb_upgrade: allow responses lager than requests When invoking vendor command handlers in try_vendor_command(), the buffer containing the command is passed to the handler to communicate the command contents and to hold the command execution return data. It was fine when invoking vendor command handlers from the TPM stack, as the receive buffer is 4K in size and is large enough for any expected vendor command response. It is different in case of USB: the command is in the receive buffer of the USB queue, and the response data could easily exceed the command size, which would cause corruption of the USB receive queue contents when the response data is placed into the same buffer where the command is. Let's introduce a local storage to pass the command and receive the response data from the handler. 32 bytes is enough for the foreseeable future, should a need arise for a larger buffer, testing would result in an error (a new error type is added to indicate insufficient buffer space for command processing). BRANCH=none BUG=b:35587387,b:35587053 TEST=with the rest of the patches applied verified proper processing of the 'Get Board ID' command for which response size exceeds the request size. Change-Id: I2131496f3a99c7f3a1869905120a453d75efbdce Signed-off-by: Vadim Bendebury Reviewed-on: https://chromium-review.googlesource.com/525092 Reviewed-by: Aseda Aboagye --- chip/g/usb_upgrade.c | 25 ++++++++++++++++++++----- include/tpm_vendor_cmds.h | 2 ++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/chip/g/usb_upgrade.c b/chip/g/usb_upgrade.c index ff1e7f4048..acbec230b7 100644 --- a/chip/g/usb_upgrade.c +++ b/chip/g/usb_upgrade.c @@ -145,6 +145,11 @@ static int try_vendor_command(struct consumer const *consumer, size_t count) uint16_t *subcommand; size_t response_size; size_t request_size; + /* + * Should be enough for any vendor command/response. We'll + * generate an error if it is not. + */ + uint8_t subcommand_body[32]; /* looks good, let's process it. */ rv = 1; @@ -156,12 +161,22 @@ static int try_vendor_command(struct consumer const *consumer, size_t count) request_size = count - sizeof(struct update_frame_header) - sizeof(*subcommand); - usb_extension_route_command(be16toh(*subcommand), - subcommand + 1, - request_size, - &response_size); + if (request_size > sizeof(subcommand_body)) { + CPRINTS("%s: vendor command payload too big (%d)", + __func__, request_size); + subcommand_body[0] = VENDOR_RC_REQUEST_TOO_BIG; + response_size = 1; + } else { + memcpy(subcommand_body, subcommand + 1, request_size); + response_size = sizeof(subcommand_body); + usb_extension_route_command(be16toh(*subcommand), + subcommand_body, + request_size, + &response_size); + } - QUEUE_ADD_UNITS(&upgrade_to_usb, subcommand + 1, response_size); + QUEUE_ADD_UNITS(&upgrade_to_usb, + subcommand_body, response_size); } shared_mem_release(cmd_buffer); diff --git a/include/tpm_vendor_cmds.h b/include/tpm_vendor_cmds.h index 4861457d36..1f9e552aaa 100644 --- a/include/tpm_vendor_cmds.h +++ b/include/tpm_vendor_cmds.h @@ -58,6 +58,8 @@ enum vendor_cmd_rc { VENDOR_RC_BOGUS_ARGS = 1, VENDOR_RC_READ_FLASH_FAIL = 2, VENDOR_RC_WRITE_FLASH_FAIL = 3, + VENDOR_RC_REQUEST_TOO_BIG = 4, + VENDOR_RC_RESPONSE_TOO_BIG = 5, /* Only 7 bits available; max is 127 */ VENDOR_RC_NO_SUCH_COMMAND = 127, };