From 9df26ce0f2a97bbc58b1807483fd22005e253b0f Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Fri, 18 May 2018 14:27:42 -0700 Subject: cr50: Refactor tracking vendor command origin Added flags parameter to extension_route_command(). The caller now specifies whether the command comes from the USB interface or the AP. Moved USB-specific shuffling of response to embed result code into usb_upgrade.c, so extension_route_command() can be more generic. No change to permissions/behavior for existing commands. ccd_command_wrapper() still sends vendor commands as if they come from the AP. That's fixed in the next CL. Reduces code size by 128 bytes BUG=b:79983505 BRANCH=cr50 TEST=manual Build with DEBUG_EXTENSION defined, to turn on printing each command 'ccd lock' comes from AP and works From host, 'gscutil -I' comes from USB and fails From AP, 'gscutil -t -I' comes from AP and works Change-Id: I7136bb54073de9c5951a174c308151b1871c56f3 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/1068101 Reviewed-by: Vadim Bendebury --- chip/g/usb_upgrade.c | 29 ++++++++++++ common/extension.c | 118 ++++++++++++++++++------------------------------- common/tpm_registers.c | 11 ++--- include/extension.h | 28 +++++++----- 4 files changed, 97 insertions(+), 89 deletions(-) diff --git a/chip/g/usb_upgrade.c b/chip/g/usb_upgrade.c index aeafd819fa..3c3982af05 100644 --- a/chip/g/usb_upgrade.c +++ b/chip/g/usb_upgrade.c @@ -106,6 +106,35 @@ static int valid_transfer_start(struct consumer const *consumer, size_t count, return 0; return 1; } + +static void usb_extension_route_command(uint16_t command_code, + uint8_t *buffer, + size_t in_size, + size_t *out_size) +{ + uint32_t rv; + size_t buf_size; + + /* + * The return code normally put into the TPM response header + * is not present in the USB response. Vendor command return + * code is guaranteed to fit in a byte. Let's keep space for + * it in the front of the buffer. + */ + buf_size = *out_size - 1; + rv = extension_route_command(command_code, buffer, in_size, &buf_size, + VENDOR_CMD_FROM_USB); + /* + * Copy actual response, if any, one byte up, to free room for + * the return code. + */ + if (buf_size) + memmove(buffer + 1, buffer, buf_size); + *out_size = buf_size + 1; + + buffer[0] = rv; /* We care about LSB only. */ +} + static int try_vendor_command(struct consumer const *consumer, size_t count) { struct update_frame_header ufh; diff --git a/common/extension.c b/common/extension.c index 610db49851..f63734025f 100644 --- a/common/extension.c +++ b/common/extension.c @@ -9,20 +9,48 @@ #include "link_defs.h" #include "util.h" -#define CPRINTF(format, args...) cprintf(CC_EXTENSION, format, ## args) +#define CPRINTS(format, args...) cprints(CC_EXTENSION, format, ## args) -static uint32_t extension_route_command(uint16_t command_code, - void *buffer, - size_t in_size, - size_t *out_size) +uint32_t extension_route_command(uint16_t command_code, + void *buffer, + size_t in_size, + size_t *out_size, + uint32_t flags) { struct extension_command *cmd_p; struct extension_command *end_p; + const char *why_ignore = "not found"; - cmd_p = (struct extension_command *)&__extension_cmds; - end_p = (struct extension_command *)&__extension_cmds_end; +#ifdef DEBUG_EXTENSION + CPRINTS("%s(%d,%s)", __func__, command_code, + flags & VENDOR_CMD_FROM_USB ? "USB" : "AP"); +#endif + + /* Filter commands from USB */ + if (flags & VENDOR_CMD_FROM_USB) { + switch (command_code) { +#ifdef CR50_DEV + case VENDOR_CC_IMMEDIATE_RESET: + case VENDOR_CC_INVALIDATE_INACTIVE_RW: + case VENDOR_CC_SET_BOARD_ID: +#endif /* defined(CR50_DEV) */ + case EXTENSION_POST_RESET: /* Always need to reset. */ + case VENDOR_CC_GET_BOARD_ID: + case VENDOR_CC_SPI_HASH: /* Requires physical presence. */ + case VENDOR_CC_TURN_UPDATE_ON: + break; + default: + /* Otherwise, we don't allow this command. */ + why_ignore = "usb"; + goto ignore_cmd; + } + } #ifdef CONFIG_BOARD_ID_SUPPORT + /* + * If board ID is mismatched, allow only the commands needed to upgrade + * Cr50 firmware. + */ if (board_id_is_mismatched()) { switch (command_code) { case EXTENSION_FW_UPGRADE: @@ -31,13 +59,15 @@ static uint32_t extension_route_command(uint16_t command_code, case EXTENSION_POST_RESET: break; default: - CPRINTF("%s: ignoring command 0x%x " - "due to board ID mismatch\n", - __func__, command_code); - return VENDOR_RC_NO_SUCH_COMMAND; + why_ignore = "BoardID mismatch"; + goto ignore_cmd; } } #endif + + /* Find the command handler */ + cmd_p = (struct extension_command *)&__extension_cmds; + end_p = (struct extension_command *)&__extension_cmds_end; while (cmd_p != end_p) { if (cmd_p->command_code == command_code) return cmd_p->handler(command_code, buffer, @@ -45,69 +75,9 @@ static uint32_t extension_route_command(uint16_t command_code, cmd_p++; } - CPRINTF("%s: handler %d not found\n", __func__, command_code); - - /* This covers the case of the handler not found. */ +ignore_cmd: + /* Command not found or not allowed */ + CPRINTS("%s: ignore %d: %s", __func__, command_code, why_ignore); *out_size = 0; return VENDOR_RC_NO_SUCH_COMMAND; } - -void usb_extension_route_command(uint16_t command_code, - void *buffer, - size_t in_size, - size_t *out_size) -{ - uint32_t rv; - uint8_t *buf = buffer; /* Cache it for easy pointer arithmetics. */ - size_t buf_size; - - switch (command_code) { -#ifdef CR50_DEV - case VENDOR_CC_IMMEDIATE_RESET: - case VENDOR_CC_INVALIDATE_INACTIVE_RW: - case VENDOR_CC_SET_BOARD_ID: -#endif /* defined(CR50_DEV) */ - case EXTENSION_POST_RESET: /* Always need to be able to reset. */ - case VENDOR_CC_GET_BOARD_ID: - case VENDOR_CC_SPI_HASH: /* This will require physical presence. */ - case VENDOR_CC_TURN_UPDATE_ON: - - /* - * The return code normally put into the TPM response header - * is not present in the USB response. Vendor command return - * code is guaranteed to fit in a byte. Let's keep space for - * it in the front of the buffer. - */ - buf_size = *out_size - 1; - rv = extension_route_command(command_code, buffer, - in_size, &buf_size); - /* - * Copy actual response, if any, one byte up, to free room for - * the return code. - */ - if (buf_size) - memmove(buf + 1, buf, buf_size); - *out_size = buf_size + 1; - break; - - default: - /* Otherwise, we don't allow this command. */ - CPRINTF("%s: ignoring vendor cmd %d\n", __func__, command_code); - *out_size = 1; - rv = VENDOR_RC_NO_SUCH_COMMAND; - break; - } - - buf[0] = rv; /* We care about LSB only. */ -} - -uint32_t tpm_extension_route_command(uint16_t command_code, - void *buffer, - size_t in_size, - size_t *out_size) -{ - /* - * TODO(aaboagye): Determine what commands (if any) should be filtered. - */ - return extension_route_command(command_code, buffer, in_size, out_size); -} diff --git a/common/tpm_registers.c b/common/tpm_registers.c index 06405d4b7e..959ca1c814 100644 --- a/common/tpm_registers.c +++ b/common/tpm_registers.c @@ -648,11 +648,12 @@ static void call_extension_command(struct tpm_cmd_header *tpmh, *total_size -= sizeof(struct tpm_cmd_header); subcommand_code = be16toh(tpmh->subcommand_code); - rc = tpm_extension_route_command(subcommand_code, - tpmh + 1, - command_size - - sizeof(struct tpm_cmd_header), - total_size); + rc = extension_route_command(subcommand_code, + tpmh + 1, + command_size - + sizeof(struct tpm_cmd_header), + total_size, + 0); /* Add the header size back. */ *total_size += sizeof(struct tpm_cmd_header); tpmh->size = htobe32(*total_size); diff --git a/include/extension.h b/include/extension.h index e5f183eb49..bec6bf3ffd 100644 --- a/include/extension.h +++ b/include/extension.h @@ -12,6 +12,16 @@ #include "common.h" #include "tpm_vendor_cmds.h" +/* Flags for vendor or extension commands */ +enum vendor_cmd_flags { + /* + * Command is coming from the USB interface (either via the vendor + * command endpoint or the console). If this flag is not present, + * the command is coming from the AP. + */ + VENDOR_CMD_FROM_USB = (1 << 0), +}; + /* * Type of function handling extension commands. * @@ -35,19 +45,17 @@ typedef enum vendor_cmd_rc (*extension_handler)(enum vendor_cmd_cc code, * @param command_code Code associated with a extension command handler. * @param buffer Data to be processd by the handler, the same space * is used for data returned by the handler. - * @command_size Size of the input data. - * @param size On input - max size of the buffer, on output - actual number of - * data returned by the handler. A single byte return + * @param in_size Size of the input data. + * @param out_size On input: max size of the buffer. On output: actual + * number of bytes returned by the handler; a single byte * usually indicates an error and contains the error code. + * @param flags Zero or more flags from vendor_cmd_flags. */ -void usb_extension_route_command(uint16_t command_code, +uint32_t extension_route_command(uint16_t command_code, void *buffer, - size_t command_size, - size_t *size); -uint32_t tpm_extension_route_command(uint16_t command_code, - void *buffer, - size_t command_size, - size_t *size); + size_t in_size, + size_t *out_size, + uint32_t flags); /* Pointer table */ struct extension_command { -- cgit v1.2.1