diff options
author | Vadim Bendebury <vbendeb@chromium.org> | 2017-06-02 16:05:36 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2017-06-06 14:36:28 -0700 |
commit | a75f7c8680afd3edbb967dbdf664e421904e6c76 (patch) | |
tree | fb60fa4c03e8a3e6f46fcc7480b1d88a1f79c8e0 /chip | |
parent | 6d05a31a4442c0475e2bcfc33692089e78c73da2 (diff) | |
download | chrome-ec-a75f7c8680afd3edbb967dbdf664e421904e6c76.tar.gz |
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 <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/525092
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
Diffstat (limited to 'chip')
-rw-r--r-- | chip/g/usb_upgrade.c | 25 |
1 files changed, 20 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); |