summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVadim Bendebury <vbendeb@chromium.org>2016-07-07 10:52:46 -0700
committerMartin Roth <martinroth@google.com>2016-07-13 23:58:32 +0200
commitbc927107a062ca791ffb4a4f593049a7219b5a07 (patch)
tree31d7196e9dc33539ddfeb3fc770ba711c49a747a
parent03e4472e179c51a43498533b90a49234e15a335b (diff)
downloadcoreboot-bc927107a062ca791ffb4a4f593049a7219b5a07.tar.xz
tpm2: avoid comparison between signed and unsigned ints
The marshaling/unmarshaling code is using integer values to represent room left in the buffer, to be able to communicate three conditions: positive number means there is room left in the buffer, zero means that the exact amount of data in the buffer was unmarshaled and negative value means that the result of the operation did not fit into the buffer. The implementation is wrong though, as it compares directly signed and unsigned values, which is illegal, as signed values get promoted to unsigned by the compiler. This patch changes the marshaling code to use size_t for the size, and use zero as marshaling failure indication - after all the buffer where the data is marshaled to should definitely be large enough, and it is reasonable to expect at least some room left in it after marshaling. The unmarshaling situation is different: we sure want to communicate errors to the caller, but do not want to propagate error return values through multiple layers. This patch keeps the size value in int, but checks if it is negative separately, before comparing with positive values. BRANCH=none BUG=chrome-os-partner:50645 TEST=with the rest of the patches applied kevin successfully boots up. Change-Id: Ibfbd1b351e35e37c8925a78d095e4e8492805bad Signed-off-by: Martin Roth <martinroth@chromium.org> Original-Commit-Id: b1e862c2a650fa5f6cb25a01fe61e848a696cf17 Original-Change-Id: Ie7552b333afaff9a1234c948caf9d9a64447b2e1 Original-Signed-off-by: Vadim Bendebury <vbendeb@chromium.org> Original-Reviewed-on: https://chromium-review.googlesource.com/358772 Original-Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/15610 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi <pgeorgi@google.com>
-rw-r--r--src/lib/tpm2_marshaling.c93
-rw-r--r--src/lib/tpm2_marshaling.h4
2 files changed, 63 insertions, 34 deletions
diff --git a/src/lib/tpm2_marshaling.c b/src/lib/tpm2_marshaling.c
index 6eba82641c..0a90df0e96 100644
--- a/src/lib/tpm2_marshaling.c
+++ b/src/lib/tpm2_marshaling.c
@@ -21,12 +21,15 @@ static uint16_t tpm_tag; /* Depends on the command type. */
* Should there be not enough data in the buffer to unmarshal the required
* object, the remaining data size is set to -1 to indicate the error. The
* remaining data size is expected to be set to zero once the last data item
- * has been extracted from the buffer.
+ * has been extracted from the receive buffer.
*/
static uint16_t unmarshal_u16(void **buffer, int *buffer_space)
{
uint16_t value;
+ if (*buffer_space < 0)
+ return 0;
+
if (*buffer_space < sizeof(value)) {
*buffer_space = -1; /* Indicate a failure. */
return 0;
@@ -43,6 +46,9 @@ static uint16_t unmarshal_u32(void **buffer, int *buffer_space)
{
uint32_t value;
+ if (*buffer_space < 0)
+ return 0;
+
if (*buffer_space < sizeof(value)) {
*buffer_space = -1; /* Indicate a failure. */
return 0;
@@ -59,6 +65,9 @@ static uint8_t unmarshal_u8(void **buffer, int *buffer_space)
{
uint8_t value;
+ if (*buffer_space < 0)
+ return 0;
+
if (*buffer_space < sizeof(value)) {
*buffer_space = -1; /* Indicate a failure. */
return 0;
@@ -87,7 +96,7 @@ static uint8_t unmarshal_u8(void **buffer, int *buffer_space)
* parameter set.
*/
static void marshal_blob(void **buffer, void *blob,
- size_t blob_size, int *buffer_space)
+ size_t blob_size, size_t *buffer_space)
{
if (*buffer_space < blob_size) {
*buffer_space = -1;
@@ -95,16 +104,16 @@ static void marshal_blob(void **buffer, void *blob,
}
memcpy(*buffer, blob, blob_size);
- buffer_space -= blob_size;
+ *buffer_space -= blob_size;
*buffer = (void *)((uintptr_t)(*buffer) + blob_size);
}
-static void marshal_u8(void **buffer, uint8_t value, int *buffer_space)
+static void marshal_u8(void **buffer, uint8_t value, size_t *buffer_space)
{
uint8_t *bp = *buffer;
if (*buffer_space < sizeof(value)) {
- *buffer_space = -1;
+ *buffer_space = 0;
return;
}
@@ -113,10 +122,10 @@ static void marshal_u8(void **buffer, uint8_t value, int *buffer_space)
*buffer_space -= sizeof(value);
}
-static void marshal_u16(void **buffer, uint16_t value, int *buffer_space)
+static void marshal_u16(void **buffer, uint16_t value, size_t *buffer_space)
{
if (*buffer_space < sizeof(value)) {
- *buffer_space = -1;
+ *buffer_space = 0;
return;
}
write_be16(*buffer, value);
@@ -124,10 +133,10 @@ static void marshal_u16(void **buffer, uint16_t value, int *buffer_space)
*buffer_space -= sizeof(value);
}
-static void marshal_u32(void **buffer, uint32_t value, int *buffer_space)
+static void marshal_u32(void **buffer, uint32_t value, size_t *buffer_space)
{
if (*buffer_space < sizeof(value)) {
- *buffer_space = -1;
+ *buffer_space = 0;
return;
}
@@ -144,14 +153,14 @@ static void marshal_u32(void **buffer, uint32_t value, int *buffer_space)
static void marshal_startup(void **buffer,
struct tpm2_startup *cmd_body,
- int *buffer_space)
+ size_t *buffer_space)
{
marshal_u16(buffer, cmd_body->startup_type, buffer_space);
}
static void marshal_get_capability(void **buffer,
struct tpm2_get_capability *cmd_body,
- int *buffer_space)
+ size_t *buffer_space)
{
marshal_u32(buffer, cmd_body->capability, buffer_space);
marshal_u32(buffer, cmd_body->property, buffer_space);
@@ -160,12 +169,12 @@ static void marshal_get_capability(void **buffer,
static void marshal_TPM2B(void **buffer,
TPM2B *data,
- int *buffer_space)
+ size_t *buffer_space)
{
size_t total_size = data->size + sizeof(data->size);
if (total_size > *buffer_space) {
- *buffer_space = -1;
+ *buffer_space = 0;
return;
}
marshal_u16(buffer, data->size, buffer_space);
@@ -176,14 +185,14 @@ static void marshal_TPM2B(void **buffer,
static void marshal_TPMA_NV(void **buffer,
TPMA_NV *nv,
- int *buffer_space)
+ size_t *buffer_space)
{
marshal_u32(buffer, *((uint32_t *)nv), buffer_space);
}
static void marshal_TPMS_NV_PUBLIC(void **buffer,
TPMS_NV_PUBLIC *nvpub,
- int *buffer_space)
+ size_t *buffer_space)
{
marshal_TPM_HANDLE(buffer, nvpub->nvIndex, buffer_space);
marshal_TPMI_ALG_HASH(buffer, nvpub->nameAlg, buffer_space);
@@ -194,12 +203,17 @@ static void marshal_TPMS_NV_PUBLIC(void **buffer,
static void marshal_session_header(void **buffer,
struct tpm2_session_header *session_header,
- int *buffer_space)
+ size_t *buffer_space)
{
- int base_size;
+ size_t base_size;
void *size_location = *buffer;
/* Skip room for the session header size. */
+ if (*buffer_space < sizeof(uint32_t)) {
+ *buffer_space = 0;
+ return;
+ }
+
*buffer_space -= sizeof(uint32_t);
*buffer = (void *)(((uintptr_t) *buffer) + sizeof(uint32_t));
@@ -214,7 +228,7 @@ static void marshal_session_header(void **buffer,
marshal_blob(buffer, session_header->auth,
session_header->auth_size, buffer_space);
- if (*buffer_space < 0)
+ if (!*buffer_space)
return; /* The structure did not fit. */
/* Paste in the session size. */
@@ -223,10 +237,11 @@ static void marshal_session_header(void **buffer,
static void marshal_nv_define_space(void **buffer,
struct tpm2_nv_define_space_cmd *nvd_in,
- int *buffer_space)
+ size_t *buffer_space)
{
void *size_location;
- int base_size;
+ size_t base_size;
+ size_t sizeof_nv_public_size = sizeof(uint16_t);
struct tpm2_session_header session_header;
marshal_TPM_HANDLE(buffer, TPM_RH_PLATFORM, buffer_space);
@@ -242,17 +257,25 @@ static void marshal_nv_define_space(void **buffer,
/* Allocate room for the size. */
*buffer = ((uint8_t *)(*buffer)) + sizeof(uint16_t);
- *buffer_space -= sizeof(uint16_t);
+
+ if (*buffer_space < sizeof_nv_public_size) {
+ *buffer_space = 0;
+ return;
+ }
+ *buffer_space -= sizeof_nv_public_size;
base_size = *buffer_space;
marshal_TPMS_NV_PUBLIC(buffer, &nvd_in->publicInfo, buffer_space);
+ if (!*buffer_space)
+ return;
+
base_size = base_size - *buffer_space;
- marshal_u16(&size_location, (uint16_t)base_size, &base_size);
+ marshal_u16(&size_location, base_size, &sizeof_nv_public_size);
}
static void marshal_nv_write(void **buffer,
struct tpm2_nv_write_cmd *command_body,
- int *buffer_space)
+ size_t *buffer_space)
{
struct tpm2_session_header session_header;
@@ -269,7 +292,7 @@ static void marshal_nv_write(void **buffer,
static void marshal_nv_read(void **buffer,
struct tpm2_nv_read_cmd *command_body,
- int *buffer_space)
+ size_t *buffer_space)
{
struct tpm2_session_header session_header;
@@ -285,17 +308,17 @@ static void marshal_nv_read(void **buffer,
static void marshal_selftest(void **buffer,
struct tpm2_self_test *command_body,
- int *buffer_space)
+ size_t *buffer_space)
{
marshal_u8(buffer, command_body->yes_no, buffer_space);
}
int tpm_marshal_command(TPM_CC command, void *tpm_command_body,
- void *buffer, int buffer_size)
+ void *buffer, size_t buffer_size)
{
void *cmd_body = (uint8_t *)buffer + sizeof(struct tpm_header);
- int max_body_size = buffer_size - sizeof(struct tpm_header);
- int body_size = max_body_size;
+ size_t max_body_size = buffer_size - sizeof(struct tpm_header);
+ size_t body_size = max_body_size;
/* Will be modified when marshaling some commands. */
tpm_tag = TPM_ST_NO_SESSIONS;
@@ -328,7 +351,7 @@ int tpm_marshal_command(TPM_CC command, void *tpm_command_body,
break;
default:
- body_size = -1;
+ body_size = 0;
printk(BIOS_INFO, "%s:%d:Request to marshal unsupported command %#x\n",
__FILE__, __LINE__, command);
}
@@ -396,10 +419,11 @@ static void unmarshal_TPM2B_MAX_NV_BUFFER(void **buffer,
TPM2B_MAX_NV_BUFFER *nv_buffer)
{
nv_buffer->t.size = unmarshal_u16(buffer, size);
- if (nv_buffer->t.size > *size) {
+ if ((*size < 0) || (nv_buffer->t.size > *size)) {
printk(BIOS_ERR, "%s:%d - "
"size mismatch: expected %d, remaining %d\n",
__func__, __LINE__, nv_buffer->t.size, *size);
+ *size = -1;
return;
}
@@ -442,11 +466,16 @@ static void unmarshal_nv_read(void **buffer, int *size,
struct tpm2_response *tpm_unmarshal_response(TPM_CC command,
void *response_body,
- int cr_size)
+ size_t in_size)
{
static struct tpm2_response tpm2_resp;
+ /*
+ * Should be 0 when done, positive and negaitive values indicate
+ * unmarshaling errors.
+ */
+ int cr_size = in_size;
- if (cr_size < sizeof(struct tpm_header))
+ if ((cr_size < 0) || (in_size < sizeof(struct tpm_header)))
return NULL;
tpm2_resp.hdr.tpm_tag = unmarshal_u16(&response_body, &cr_size);
diff --git a/src/lib/tpm2_marshaling.h b/src/lib/tpm2_marshaling.h
index 69a345d731..e177d0609a 100644
--- a/src/lib/tpm2_marshaling.h
+++ b/src/lib/tpm2_marshaling.h
@@ -25,7 +25,7 @@
*
*/
int tpm_marshal_command(TPM_CC command, void *tpm_command_body,
- void *buffer, int buffer_size);
+ void *buffer, size_t buffer_size);
/**
* tpm_unmarshal_response
@@ -44,6 +44,6 @@ int tpm_marshal_command(TPM_CC command, void *tpm_command_body,
*/
struct tpm2_response *tpm_unmarshal_response(TPM_CC command,
void *response_body,
- int response_size);
+ size_t response_size);
#endif // __SRC_LIB_TPM2_MARSHALING_H