[coreboot-gerrit] Change in coreboot[master]: drivers/i2c/tpm: use iobuf library for marshaling commands

Martin Roth (Code Review) gerrit at coreboot.org
Mon Apr 24 19:07:09 CEST 2017


Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/19063 )

Change subject: drivers/i2c/tpm: use iobuf library for marshaling commands
......................................................................


drivers/i2c/tpm: use iobuf library for marshaling commands

Use the iobuf API instead of relying on own buffer management. It
also provides consistency between marshaling and unmarshaling code
paths for propagating return values instead of overloading the values
of existing variables.

BUG=b:36598499

Change-Id: Iec0bbff1312e8e6ec616d1528db8667f32e682c9
Signed-off-by: Aaron Durbin <adurbin at chromium.org>
Reviewed-on: https://review.coreboot.org/19063
Tested-by: build bot (Jenkins)
Reviewed-by: Furquan Shaikh <furquan at google.com>
---
M src/lib/tpm2_marshaling.c
M src/lib/tpm2_marshaling.h
M src/lib/tpm2_tlcl.c
3 files changed, 274 insertions(+), 415 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved



diff --git a/src/lib/tpm2_marshaling.c b/src/lib/tpm2_marshaling.c
index 52dac9c..3ce09a4 100644
--- a/src/lib/tpm2_marshaling.c
+++ b/src/lib/tpm2_marshaling.c
@@ -5,7 +5,7 @@
  */
 
 #include <arch/early_variables.h>
-#include <commonlib/endian.h>
+#include <commonlib/iobuf.h>
 #include <console/console.h>
 #include <stdlib.h>
 #include <string.h>
@@ -14,522 +14,378 @@
 
 static uint16_t tpm_tag CAR_GLOBAL;  /* Depends on the command type. */
 
-/*
- * Each unmarshaling function receives a pointer to the buffer pointer and a
- * pointer to the size of data still in the buffer. The function extracts data
- * from the buffer and adjusts both buffer pointer and remaining data size.
- *
- * 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 receive buffer.
- */
-static uint16_t unmarshal_u16(void **buffer, int *buffer_space)
+#define unmarshal_TPM_CAP(a, b) ibuf_read_be32(a, b)
+#define unmarshal_TPM_CC(a, b) ibuf_read_be32(a, b)
+#define unmarshal_TPM_PT(a, b) ibuf_read_be32(a, b)
+#define unmarshal_TPM_HANDLE(a, b) ibuf_read_be32(a, b)
+
+#define marshal_TPM_HANDLE(a, b) obuf_write_be32(a, b)
+#define marshal_TPMI_ALG_HASH(a, b) obuf_write_be16(a, b)
+
+static int marshal_startup(struct obuf *ob, struct tpm2_startup *cmd_body)
 {
-	uint16_t value;
-
-	if (*buffer_space < 0)
-		return 0;
-
-	if (*buffer_space < sizeof(value)) {
-		*buffer_space = -1; /* Indicate a failure. */
-		return 0;
-	}
-
-	value = read_be16(*buffer);
-	*buffer = (void *) ((uintptr_t) (*buffer) + sizeof(value));
-	*buffer_space -= sizeof(value);
-
-	return value;
+	return obuf_write_be16(ob, cmd_body->startup_type);
 }
 
-static uint16_t unmarshal_u32(void **buffer, int *buffer_space)
+static int marshal_get_capability(struct obuf *ob,
+				   struct tpm2_get_capability *cmd_body)
 {
-	uint32_t value;
+	int rc = 0;
 
-	if (*buffer_space < 0)
-		return 0;
+	rc |= obuf_write_be32(ob, cmd_body->capability);
+	rc |= obuf_write_be32(ob, cmd_body->property);
+	rc |= obuf_write_be32(ob, cmd_body->propertyCount);
 
-	if (*buffer_space < sizeof(value)) {
-		*buffer_space = -1; /* Indicate a failure. */
-		return 0;
-	}
-
-	value = read_be32(*buffer);
-	*buffer = (void *) ((uintptr_t) (*buffer) + sizeof(value));
-	*buffer_space -= sizeof(value);
-
-	return value;
+	return rc;
 }
 
-static uint8_t unmarshal_u8(void **buffer, int *buffer_space)
+static int marshal_TPM2B(struct obuf *ob, TPM2B *data)
 {
-	uint8_t value;
+	int rc = 0;
 
-	if (*buffer_space < 0)
-		return 0;
+	rc |= obuf_write_be16(ob, data->size);
+	rc |= obuf_write(ob, data->buffer, data->size);
 
-	if (*buffer_space < sizeof(value)) {
-		*buffer_space = -1; /* Indicate a failure. */
-		return 0;
-	}
-
-	value = ((uint8_t *)(*buffer))[0];
-	*buffer = (void *) ((uintptr_t) (*buffer) + sizeof(value));
-	*buffer_space -= sizeof(value);
-
-	return value;
+	return rc;
 }
 
-#define unmarshal_TPM_CAP(a, b) unmarshal_u32(a, b)
-#define unmarshal_TPM_CC(a, b) unmarshal_u32(a, b)
-#define unmarshal_TPM_PT(a, b) unmarshal_u32(a, b)
-#define unmarshal_TPM_HANDLE(a, b) unmarshal_u32(a, b)
-
-/*
- * Each marshaling function receives a pointer to the buffer to marshal into,
- * a pointer to the data item to be marshaled, and a pointer to the remaining
- * room in the buffer.
- */
-
- /*
-  * Marshaling an arbitrary blob requires its size in addition to common
-  * parameter set.
-  */
-static void marshal_blob(void **buffer, void *blob,
-			 size_t blob_size, size_t *buffer_space)
+static int marshal_TPMA_NV(struct obuf *ob, TPMA_NV *nv)
 {
-	if (*buffer_space < blob_size) {
-		*buffer_space = 0;
-		return;
-	}
+	uint32_t v;
 
-	memcpy(*buffer, blob, blob_size);
-	*buffer_space -= blob_size;
-	*buffer = (void *)((uintptr_t)(*buffer) + blob_size);
+	memcpy(&v, nv, sizeof(v));
+	return obuf_write_be32(ob, v);
 }
 
-static void marshal_u8(void **buffer, uint8_t value, size_t *buffer_space)
+static int marshal_TPMS_NV_PUBLIC(struct obuf *ob, TPMS_NV_PUBLIC *nvpub)
 {
-	uint8_t *bp = *buffer;
+	int rc = 0;
 
-	if (*buffer_space < sizeof(value)) {
-		*buffer_space = 0;
-		return;
-	}
+	rc |= marshal_TPM_HANDLE(ob, nvpub->nvIndex);
+	rc |= marshal_TPMI_ALG_HASH(ob, nvpub->nameAlg);
+	rc |= marshal_TPMA_NV(ob, &nvpub->attributes);
+	rc |= marshal_TPM2B(ob, &nvpub->authPolicy.b);
+	rc |= obuf_write_be16(ob, nvpub->dataSize);
 
-	*bp++ = value;
-	*buffer = bp;
-	*buffer_space -= sizeof(value);
+	return rc;
 }
 
-static void marshal_u16(void **buffer, uint16_t value, size_t *buffer_space)
+static int marshal_TPMT_HA(struct obuf *ob, TPMT_HA *tpmtha)
 {
-	if (*buffer_space < sizeof(value)) {
-		*buffer_space = 0;
-		return;
-	}
-	write_be16(*buffer, value);
-	*buffer = (void *)((uintptr_t)(*buffer) + sizeof(value));
-	*buffer_space -= sizeof(value);
+	int rc = 0;
+
+	rc |= marshal_TPMI_ALG_HASH(ob, tpmtha->hashAlg);
+	rc |= obuf_write(ob, tpmtha->digest.sha256,
+			sizeof(tpmtha->digest.sha256));
+
+	return rc;
 }
 
-static void marshal_u32(void **buffer, uint32_t value, size_t *buffer_space)
-{
-	if (*buffer_space < sizeof(value)) {
-		*buffer_space = 0;
-		return;
-	}
-
-	write_be32(*buffer, value);
-	*buffer = (void *)((uintptr_t)(*buffer) + sizeof(value));
-	*buffer_space -= sizeof(value);
-}
-
-#define marshal_TPM_HANDLE(a, b, c) marshal_u32(a, b, c)
-#define marshal_TPMI_RH_NV_AUTH(a, b, c) marshal_TPM_HANDLE(a, b, c)
-#define marshal_TPMI_RH_NV_INDEX(a, b, c) marshal_TPM_HANDLE(a, b, c)
-#define marshal_TPMI_SH_AUTH_SESSION(a, b, c) marshal_TPM_HANDLE(a, b, c)
-#define marshal_TPMI_ALG_HASH(a, b, c) marshal_u16(a, b, c)
-
-static void marshal_startup(void **buffer,
-			   struct tpm2_startup *cmd_body,
-			   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,
-				   size_t *buffer_space)
-{
-	marshal_u32(buffer, cmd_body->capability, buffer_space);
-	marshal_u32(buffer, cmd_body->property, buffer_space);
-	marshal_u32(buffer, cmd_body->propertyCount, buffer_space);
-}
-
-static void marshal_TPM2B(void **buffer,
-			  TPM2B *data,
-			  size_t *buffer_space)
-{
-	size_t total_size = data->size + sizeof(data->size);
-
-	if (total_size > *buffer_space) {
-		*buffer_space = 0;
-		return;
-	}
-	marshal_u16(buffer, data->size, buffer_space);
-	memcpy(*buffer, data->buffer, data->size);
-	*buffer = ((uint8_t *)(*buffer)) + data->size;
-	*buffer_space -= data->size;
-}
-
-static void marshal_TPMA_NV(void **buffer,
-			    TPMA_NV *nv,
-			    size_t *buffer_space)
-{
-	marshal_u32(buffer, *((uint32_t *)nv), buffer_space);
-}
-
-static void marshal_TPMS_NV_PUBLIC(void **buffer,
-				   TPMS_NV_PUBLIC *nvpub,
-				   size_t *buffer_space)
-{
-	marshal_TPM_HANDLE(buffer, nvpub->nvIndex, buffer_space);
-	marshal_TPMI_ALG_HASH(buffer, nvpub->nameAlg, buffer_space);
-	marshal_TPMA_NV(buffer, &nvpub->attributes, buffer_space);
-	marshal_TPM2B(buffer, &nvpub->authPolicy.b, buffer_space);
-	marshal_u16(buffer, nvpub->dataSize, buffer_space);
-}
-
-static void marshal_TPMT_HA(void **buffer,
-			    TPMT_HA *tpmtha,
-			    size_t *buffer_space)
-{
-	marshal_TPMI_ALG_HASH(buffer, tpmtha->hashAlg, buffer_space);
-	marshal_blob(buffer, tpmtha->digest.sha256,
-		     sizeof(tpmtha->digest.sha256),
-		     buffer_space);
-}
-
-static void marshal_TPML_DIGEST_VALUES(void **buffer,
-				       TPML_DIGEST_VALUES *dvalues,
-				       size_t *buffer_space)
+static int marshal_TPML_DIGEST_VALUES(struct obuf *ob,
+				       TPML_DIGEST_VALUES *dvalues)
 {
 	int i;
+	int rc = 0;
 
-	marshal_u32(buffer, dvalues->count, buffer_space);
+	rc |= obuf_write_be32(ob, dvalues->count);
 	for (i = 0; i < dvalues->count; i++)
-		marshal_TPMT_HA(buffer, &dvalues->digests[i], buffer_space);
+		rc |= marshal_TPMT_HA(ob, &dvalues->digests[i]);
+
+	return rc;
 }
 
-static void marshal_session_header(void **buffer,
-				   struct tpm2_session_header *session_header,
-				   size_t *buffer_space)
+static int marshal_session_header(struct obuf *ob,
+				   struct tpm2_session_header *session_header)
 {
-	size_t base_size;
-	void *size_location = *buffer;
+	int rc = 0;
+	struct obuf ob_sz;
+	size_t prev_written;
 
-	/* Skip room for the session header size. */
-	if (*buffer_space < sizeof(uint32_t)) {
-		*buffer_space = 0;
-		return;
-	}
+	/* Snapshot current location to place size of header. */
+	if (obuf_splice_current(ob, &ob_sz, sizeof(uint32_t)) < 0)
+		return -1;
 
-	*buffer_space -= sizeof(uint32_t);
-	*buffer = (void *)(((uintptr_t) *buffer) + sizeof(uint32_t));
+	/* Write a size placeholder. */
+	rc |= obuf_write_be32(ob, 0);
 
-	base_size = *buffer_space;
+	/* Keep track of session header data size by tracking num written. */
+	prev_written = obuf_nr_written(ob);
 
-	marshal_u32(buffer, session_header->session_handle, buffer_space);
-	marshal_u16(buffer, session_header->nonce_size, buffer_space);
-	marshal_blob(buffer, session_header->nonce,
-		     session_header->nonce_size, buffer_space);
-	marshal_u8(buffer, session_header->session_attrs, buffer_space);
-	marshal_u16(buffer, session_header->auth_size, buffer_space);
-	marshal_blob(buffer, session_header->auth,
-		     session_header->auth_size, buffer_space);
+	rc |= obuf_write_be32(ob, session_header->session_handle);
+	rc |= obuf_write_be16(ob, session_header->nonce_size);
+	rc |= obuf_write(ob, session_header->nonce, session_header->nonce_size);
+	rc |= obuf_write_be8(ob, session_header->session_attrs);
+	rc |= obuf_write_be16(ob, session_header->auth_size);
+	rc |= obuf_write(ob, session_header->auth, session_header->auth_size);
 
-	if (!*buffer_space)
-		return;  /* The structure did not fit. */
+	/* Fill back in proper size of session header. */
+	rc |= obuf_write_be32(&ob_sz, obuf_nr_written(ob) - prev_written);
 
-	/* Paste in the session size. */
-	marshal_u32(&size_location, base_size - *buffer_space, &base_size);
+	return rc;
 }
 
 /*
  * Common session header can include one or two handles and an empty
  * session_header structure.
  */
-static void marshal_common_session_header(void **buffer,
+static int marshal_common_session_header(struct obuf *ob,
 					  const uint32_t *handles,
-					  size_t handle_count,
-					  size_t *buffer_space)
+					  size_t handle_count)
 {
-	int i;
+	size_t i;
 	struct tpm2_session_header session_header;
+	int rc = 0;
 
 	car_set_var(tpm_tag, TPM_ST_SESSIONS);
 
 	for (i = 0; i < handle_count; i++)
-		marshal_TPM_HANDLE(buffer, handles[i], buffer_space);
+		rc |= marshal_TPM_HANDLE(ob, handles[i]);
 
 	memset(&session_header, 0, sizeof(session_header));
 	session_header.session_handle = TPM_RS_PW;
-	marshal_session_header(buffer, &session_header, buffer_space);
+	rc |= marshal_session_header(ob, &session_header);
+
+	return rc;
 }
 
-static void marshal_nv_define_space(void **buffer,
-				    struct tpm2_nv_define_space_cmd *nvd_in,
-				    size_t *buffer_space)
+static int marshal_nv_define_space(struct obuf *ob,
+				    struct tpm2_nv_define_space_cmd *nvd_in)
 {
-	void *size_location;
-	size_t base_size;
-	size_t sizeof_nv_public_size = sizeof(uint16_t);
 	const uint32_t handle[] = { TPM_RH_PLATFORM };
+	struct obuf ob_sz;
+	size_t prev_written;
+	int rc = 0;
 
-	marshal_common_session_header(buffer, handle,
-				      ARRAY_SIZE(handle), buffer_space);
-	marshal_TPM2B(buffer, &nvd_in->auth.b, buffer_space);
+	rc |= marshal_common_session_header(ob, handle, ARRAY_SIZE(handle));
+	rc |= marshal_TPM2B(ob, &nvd_in->auth.b);
 
-	/* This is where the TPMS_NV_PUBLIC size will be stored. */
-	size_location = *buffer;
+	/* Snapshot current location to place size field. */
+	if (obuf_splice_current(ob, &ob_sz, sizeof(uint16_t)) < 0)
+		return -1;
 
-	/* Allocate room for the size. */
-	*buffer = ((uint8_t *)(*buffer)) + sizeof(uint16_t);
+	/* Put placeholder for size */
+	rc |= obuf_write_be16(ob, 0);
 
-	if (*buffer_space < sizeof_nv_public_size) {
-		*buffer_space = 0;
-		return;
-	}
-	*buffer_space -= sizeof_nv_public_size;
-	base_size = *buffer_space;
+	/* Keep track of nv define space data size by tracking num written. */
+	prev_written = obuf_nr_written(ob);
 
-	marshal_TPMS_NV_PUBLIC(buffer, &nvd_in->publicInfo, buffer_space);
-	if (!*buffer_space)
-		return;
+	rc |= marshal_TPMS_NV_PUBLIC(ob, &nvd_in->publicInfo);
+	rc |= obuf_write_be16(&ob_sz, obuf_nr_written(ob) - prev_written);
 
-	base_size = base_size - *buffer_space;
-	marshal_u16(&size_location, base_size, &sizeof_nv_public_size);
+	return rc;
 }
 
-static void marshal_nv_write(void **buffer,
-			     struct tpm2_nv_write_cmd *command_body,
-			     size_t *buffer_space)
+static int marshal_nv_write(struct obuf *ob,
+			     struct tpm2_nv_write_cmd *command_body)
+{
+	int rc = 0;
+	uint32_t handles[] = { TPM_RH_PLATFORM, command_body->nvIndex };
+
+	rc |= marshal_common_session_header(ob, handles, ARRAY_SIZE(handles));
+	rc |= marshal_TPM2B(ob, &command_body->data.b);
+	rc |= obuf_write_be16(ob, command_body->offset);
+
+	return rc;
+}
+
+static int marshal_nv_write_lock(struct obuf *ob,
+				  struct tpm2_nv_write_lock_cmd *command_body)
 {
 	uint32_t handles[] = { TPM_RH_PLATFORM, command_body->nvIndex };
 
-	marshal_common_session_header(buffer, handles,
-				      ARRAY_SIZE(handles), buffer_space);
-	marshal_TPM2B(buffer, &command_body->data.b, buffer_space);
-	marshal_u16(buffer, command_body->offset, buffer_space);
+	return marshal_common_session_header(ob, handles, ARRAY_SIZE(handles));
 }
 
-static void marshal_nv_write_lock(void **buffer,
-				  struct tpm2_nv_write_lock_cmd *command_body,
-				  size_t *buffer_space)
+static int marshal_pcr_extend(struct obuf *ob,
+			       struct tpm2_pcr_extend_cmd *command_body)
 {
-	uint32_t handles[] = { TPM_RH_PLATFORM, command_body->nvIndex };
-	marshal_common_session_header(buffer, handles,
-				      ARRAY_SIZE(handles), buffer_space);
+	int rc = 0;
+	uint32_t handles[] = { command_body->pcrHandle };
+
+	rc |= marshal_common_session_header(ob, handles, ARRAY_SIZE(handles));
+	rc |= marshal_TPML_DIGEST_VALUES(ob, &command_body->digests);
+
+	return rc;
 }
 
-static void marshal_pcr_extend(void **buffer,
-			       struct tpm2_pcr_extend_cmd *command_body,
-			       size_t *buffer_space)
+static int marshal_nv_read(struct obuf *ob,
+			    struct tpm2_nv_read_cmd *command_body)
 {
-	uint32_t handle = command_body->pcrHandle;
-
-	marshal_common_session_header(buffer, &handle, 1, buffer_space);
-	marshal_TPML_DIGEST_VALUES(buffer,
-				   &command_body->digests, buffer_space);
-}
-
-static void marshal_nv_read(void **buffer,
-			    struct tpm2_nv_read_cmd *command_body,
-			    size_t *buffer_space)
-{
+	int rc = 0;
 	uint32_t handles[] = { TPM_RH_PLATFORM, command_body->nvIndex };
 
-	marshal_common_session_header(buffer, handles,
-				      ARRAY_SIZE(handles), buffer_space);
-	marshal_u16(buffer, command_body->size, buffer_space);
-	marshal_u16(buffer, command_body->offset, buffer_space);
+	rc |= marshal_common_session_header(ob, handles, ARRAY_SIZE(handles));
+	rc |= obuf_write_be16(ob, command_body->size);
+	rc |= obuf_write_be16(ob, command_body->offset);
+
+	return rc;
 }
 
 /* TPM2_Clear command does not require paramaters. */
-static void marshal_clear(void **buffer, size_t *buffer_space)
+static int marshal_clear(struct obuf *ob)
 {
 	const uint32_t handle[] = { TPM_RH_PLATFORM };
 
-	marshal_common_session_header(buffer, handle,
-				      ARRAY_SIZE(handle), buffer_space);
+	return marshal_common_session_header(ob, handle, ARRAY_SIZE(handle));
 }
 
-static void marshal_selftest(void **buffer,
-			     struct tpm2_self_test *command_body,
-			     size_t *buffer_space)
+static int marshal_selftest(struct obuf *ob,
+			     struct tpm2_self_test *command_body)
 {
-	marshal_u8(buffer, command_body->yes_no, buffer_space);
+	return obuf_write_be8(ob, command_body->yes_no);
 }
 
-static void marshal_hierarchy_control(void **buffer,
-			struct tpm2_hierarchy_control_cmd *command_body,
-			size_t *buffer_space)
+static int marshal_hierarchy_control(struct obuf *ob,
+			struct tpm2_hierarchy_control_cmd *command_body)
 {
+	int rc = 0;
 	struct tpm2_session_header session_header;
 
 	car_set_var(tpm_tag, TPM_ST_SESSIONS);
 
-	marshal_TPM_HANDLE(buffer, TPM_RH_PLATFORM, buffer_space);
+	rc |= marshal_TPM_HANDLE(ob, TPM_RH_PLATFORM);
 	memset(&session_header, 0, sizeof(session_header));
 	session_header.session_handle = TPM_RS_PW;
-	marshal_session_header(buffer, &session_header, buffer_space);
+	rc |= marshal_session_header(ob, &session_header);
 
-	marshal_TPM_HANDLE(buffer, command_body->enable, buffer_space);
-	marshal_u8(buffer, command_body->state, buffer_space);
+	rc |= marshal_TPM_HANDLE(ob, command_body->enable);
+	rc |= obuf_write_be8(ob, command_body->state);
+
+	return rc;
 }
 
-static void marshal_cr50_vendor_command(void **buffer, void *command_body,
-			size_t *buffer_space)
+static int marshal_cr50_vendor_command(struct obuf *ob, void *command_body)
 {
-	uint16_t *sub_command;
-	sub_command = command_body;
+	int rc = 0;
+	uint16_t *sub_command = command_body;
 
 	switch (*sub_command) {
 	case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS:
-		marshal_u16(buffer, *sub_command, buffer_space);
+		rc |= obuf_write_be16(ob, *sub_command);
 		break;
 	case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON:
-		marshal_u16(buffer, sub_command[0], buffer_space);
-		marshal_u16(buffer, sub_command[1], buffer_space);
+		rc |= obuf_write_be16(ob, sub_command[0]);
+		rc |= obuf_write_be16(ob, sub_command[1]);
 		break;
 	default:
 		/* Unsupported subcommand. */
 		printk(BIOS_WARNING, "Unsupported cr50 subcommand: 0x%04x\n",
 			*sub_command);
-		*buffer_space = 0;
+		rc = -1;
 		break;
 	}
+	return rc;
 }
 
-int tpm_marshal_command(TPM_CC command, void *tpm_command_body,
-			void *buffer, size_t buffer_size)
+int tpm_marshal_command(TPM_CC command, void *tpm_command_body, struct obuf *ob)
 {
-	void *cmd_body = (uint8_t *)buffer + sizeof(struct tpm_header);
-	size_t max_body_size = buffer_size - sizeof(struct tpm_header);
-	size_t body_size = max_body_size;
+	struct obuf ob_hdr;
+	const size_t hdr_sz = sizeof(uint16_t) + 2 * sizeof(uint32_t);
+	int rc = 0;
 
-	/* Will be modified when marshaling some commands. */
 	car_set_var(tpm_tag, TPM_ST_NO_SESSIONS);
+
+	if (obuf_splice_current(ob, &ob_hdr, hdr_sz) < 0)
+		return -1;
+
+	/* Write TPM command header with placeholder field values. */
+	rc |= obuf_write_be16(ob, 0);
+	rc |= obuf_write_be32(ob, 0);
+	rc |= obuf_write_be32(ob, command);
+
+	if (rc != 0)
+		return rc;
 
 	switch (command) {
 	case TPM2_Startup:
-		marshal_startup(&cmd_body, tpm_command_body, &body_size);
+		rc |= marshal_startup(ob, tpm_command_body);
 		break;
 
 	case TPM2_GetCapability:
-		marshal_get_capability(&cmd_body, tpm_command_body,
-				       &body_size);
+		rc |= marshal_get_capability(ob, tpm_command_body);
 		break;
 
 	case TPM2_NV_Read:
-		marshal_nv_read(&cmd_body, tpm_command_body, &body_size);
+		rc |= marshal_nv_read(ob, tpm_command_body);
 		break;
 
 	case TPM2_NV_DefineSpace:
-		marshal_nv_define_space(&cmd_body,
-					tpm_command_body, &body_size);
+		rc |= marshal_nv_define_space(ob, tpm_command_body);
 		break;
 
 	case TPM2_NV_Write:
-		marshal_nv_write(&cmd_body, tpm_command_body, &body_size);
+		rc |= marshal_nv_write(ob, tpm_command_body);
 		break;
 
 	case TPM2_NV_WriteLock:
-		marshal_nv_write_lock(&cmd_body, tpm_command_body, &body_size);
+		rc |= marshal_nv_write_lock(ob, tpm_command_body);
 		break;
 
 	case TPM2_SelfTest:
-		marshal_selftest(&cmd_body, tpm_command_body, &body_size);
+		rc |= marshal_selftest(ob, tpm_command_body);
 		break;
 
 	case TPM2_Hierarchy_Control:
-		marshal_hierarchy_control(&cmd_body, tpm_command_body,
-						&body_size);
+		rc |= marshal_hierarchy_control(ob, tpm_command_body);
 		break;
 
 	case TPM2_Clear:
-		marshal_clear(&cmd_body, &body_size);
+		rc |= marshal_clear(ob);
 		break;
 
 	case TPM2_PCR_Extend:
-		marshal_pcr_extend(&cmd_body, tpm_command_body, &body_size);
+		rc |= marshal_pcr_extend(ob, tpm_command_body);
 		break;
 
 	case TPM2_CR50_VENDOR_COMMAND:
-		marshal_cr50_vendor_command(&cmd_body, tpm_command_body,
-			&body_size);
+		rc |= marshal_cr50_vendor_command(ob, tpm_command_body);
 		break;
 
 	default:
-		body_size = 0;
 		printk(BIOS_INFO, "%s:%d:Request to marshal unsupported command %#x\n",
 		       __FILE__, __LINE__, command);
+		rc = -1;
 	}
 
-	if (body_size > 0) {
-		size_t marshaled_size;
-		size_t header_room = sizeof(struct tpm_header);
+	if (rc != 0)
+		return rc;
 
-		/* See how much room was taken by marshaling. */
-		marshaled_size = max_body_size - body_size;
+	/* Fix up the command header with known values. */
+	rc |= obuf_write_be16(&ob_hdr, car_get_var(tpm_tag));
+	rc |= obuf_write_be32(&ob_hdr, obuf_nr_written(ob));
 
-		/* Total size includes the header size. */
-		marshaled_size += sizeof(struct tpm_header);
-
-		uint16_t tpm_tag_value = car_get_var(tpm_tag);
-
-		marshal_u16(&buffer, tpm_tag_value, &header_room);
-		marshal_u32(&buffer, marshaled_size, &header_room);
-		marshal_u32(&buffer, command, &header_room);
-		return marshaled_size;
-	}
-
-	return -1;
+	return rc;
 }
 
-static void unmarshal_get_capability(void **buffer, int *size,
+static int unmarshal_get_capability(struct ibuf *ib,
 				     struct get_cap_response *gcr)
 {
 	int i;
+	int rc = 0;
 
-	gcr->more_data = unmarshal_u8(buffer, size);
+	rc |= ibuf_read_be8(ib, &gcr->more_data);
+	rc |= unmarshal_TPM_CAP(ib, &gcr->cd.capability);
 
-	gcr->cd.capability = unmarshal_TPM_CAP(buffer, size);
-	if (*size < 0)
-		return;
+	if (rc != 0)
+		return rc;
 
 	switch (gcr->cd.capability) {
 	case TPM_CAP_TPM_PROPERTIES:
-		gcr->cd.data.tpmProperties.count =
-			unmarshal_u32(buffer, size);
-		if (*size < 0)
-			return;
+		if (ibuf_read_be32(ib, &gcr->cd.data.tpmProperties.count))
+			return -1;
 		if (gcr->cd.data.tpmProperties.count > ARRAY_SIZE
 		    (gcr->cd.data.tpmProperties.tpmProperty)) {
 			printk(BIOS_INFO, "%s:%s:%d - %d - too many properties\n",
 			       __FILE__, __func__, __LINE__,
 			       gcr->cd.data.tpmProperties.count);
-			*size = -1;
-			return;
+			return -1;
 		}
 		for (i = 0; i < gcr->cd.data.tpmProperties.count; i++) {
 			TPMS_TAGGED_PROPERTY *pp;
 
 			pp = gcr->cd.data.tpmProperties.tpmProperty + i;
-			pp->property = unmarshal_TPM_PT(buffer, size);
-			pp->value = unmarshal_u32(buffer, size);
+			rc |= unmarshal_TPM_PT(ib, &pp->property);
+			rc |= ibuf_read_be32(ib, &pp->value);
 		}
 		break;
 	default:
@@ -537,36 +393,40 @@
 		       "%s:%d - unable to unmarshal capability response",
 		       __func__, __LINE__);
 		printk(BIOS_ERR, " for %d\n", gcr->cd.capability);
-		*size = -1;
+		rc = -1;
 		break;
 	}
+
+	return rc;
 }
 
-static void unmarshal_TPM2B_MAX_NV_BUFFER(void **buffer,
-					  int *size,
+static int unmarshal_TPM2B_MAX_NV_BUFFER(struct ibuf *ib,
 					  TPM2B_MAX_NV_BUFFER *nv_buffer)
 {
-	nv_buffer->t.size = unmarshal_u16(buffer, size);
-	if ((*size < 0) || (nv_buffer->t.size > *size)) {
+	if (ibuf_read_be16(ib, &nv_buffer->t.size))
+		return -1;
+
+	nv_buffer->t.buffer = ibuf_oob_drain(ib, nv_buffer->t.size);
+
+	if (nv_buffer->t.buffer == NULL) {
 		printk(BIOS_ERR, "%s:%d - "
-		       "size mismatch: expected %d, remaining %d\n",
-		       __func__, __LINE__, nv_buffer->t.size, *size);
-		*size = -1;
-		return;
+		       "size mismatch: expected %d, remaining %zd\n",
+		       __func__, __LINE__, nv_buffer->t.size,
+			ibuf_remaining(ib));
+		return -1;
 	}
 
-	nv_buffer->t.buffer = *buffer;
-
-	*buffer = ((uint8_t *)(*buffer)) + nv_buffer->t.size;
-	*size -= nv_buffer->t.size;
+	return 0;
 }
 
-static void unmarshal_nv_read(void **buffer, int *size,
-			      struct nv_read_response *nvr)
+static int unmarshal_nv_read(struct ibuf *ib, struct nv_read_response *nvr)
 {
 	/* Total size of the parameter field. */
-	nvr->params_size = unmarshal_u32(buffer, size);
-	unmarshal_TPM2B_MAX_NV_BUFFER(buffer, size, &nvr->buffer);
+	if (ibuf_read_be32(ib, &nvr->params_size))
+		return -1;
+
+	if (unmarshal_TPM2B_MAX_NV_BUFFER(ib, &nvr->buffer))
+		return -1;
 
 	if (nvr->params_size !=
 	    (nvr->buffer.t.size + sizeof(nvr->buffer.t.size))) {
@@ -574,64 +434,60 @@
 		       "%s:%d - parameter/buffer %d/%d size mismatch",
 		       __func__, __LINE__, nvr->params_size,
 		       nvr->buffer.t.size);
-		return;
+		return -1;
 	}
 
-	if (*size < 0)
-		return;
 	/*
 	 * Let's ignore the authorisation section. It should be 5 bytes total,
 	 * just confirm that this is the case and report any discrepancy.
 	 */
-	if (*size != 5)
+	if (ibuf_remaining(ib) != 5)
 		printk(BIOS_ERR,
-		       "%s:%d - unexpected authorisation seciton size %d\n",
-		       __func__, __LINE__, *size);
+		       "%s:%d - unexpected authorisation seciton size %zd\n",
+		       __func__, __LINE__, ibuf_remaining(ib));
 
-	*buffer = ((uint8_t *)(*buffer)) + *size;
-	*size = 0;
+	ibuf_oob_drain(ib, ibuf_remaining(ib));
+
+	return 0;
 }
 
-static void unmarshal_vendor_command(void **buffer, int *size,
+static int unmarshal_vendor_command(struct ibuf *ib,
 				     struct vendor_command_response *vcr)
 {
-	vcr->vc_subcommand = unmarshal_u16(buffer, size);
+	if (ibuf_read_be16(ib, &vcr->vc_subcommand))
+		return -1;
 
 	switch (vcr->vc_subcommand) {
 	case TPM2_CR50_SUB_CMD_NVMEM_ENABLE_COMMITS:
 		break;
 	case TPM2_CR50_SUB_CMD_TURN_UPDATE_ON:
-		vcr->num_restored_headers = unmarshal_u8(buffer, size);
+		return ibuf_read_be8(ib, &vcr->num_restored_headers);
 		break;
 	default:
 		printk(BIOS_ERR,
 		       "%s:%d - unsupported vendor command %#04x!\n",
 		       __func__, __LINE__, vcr->vc_subcommand);
-		break;
+		return -1;
 	}
+
+	return 0;
 }
 
-struct tpm2_response *tpm_unmarshal_response(TPM_CC command,
-					     void *response_body,
-					     size_t in_size)
+struct tpm2_response *tpm_unmarshal_response(TPM_CC command, struct ibuf *ib)
 {
 	static struct tpm2_response tpm2_static_resp CAR_GLOBAL;
 	struct tpm2_response *tpm2_resp = car_get_var_ptr(&tpm2_static_resp);
-	/*
-	 * Should be 0 when done, positive and negaitive values indicate
-	 * unmarshaling errors.
-	 */
-	int cr_size = in_size;
+	int rc = 0;
 
-	if ((cr_size < 0) || (in_size < sizeof(struct tpm_header)))
+	rc |= ibuf_read_be16(ib, &tpm2_resp->hdr.tpm_tag);
+	rc |= ibuf_read_be32(ib, &tpm2_resp->hdr.tpm_size);
+	rc |= unmarshal_TPM_CC(ib, &tpm2_resp->hdr.tpm_code);
+
+	if (rc != 0)
 		return NULL;
 
-	tpm2_resp->hdr.tpm_tag = unmarshal_u16(&response_body, &cr_size);
-	tpm2_resp->hdr.tpm_size = unmarshal_u32(&response_body, &cr_size);
-	tpm2_resp->hdr.tpm_code = unmarshal_TPM_CC(&response_body, &cr_size);
-
-	if (!cr_size) {
-		if (tpm2_resp->hdr.tpm_size != sizeof(tpm2_resp->hdr))
+	if (ibuf_remaining(ib) == 0) {
+		if (tpm2_resp->hdr.tpm_size != ibuf_nr_read(ib))
 			printk(BIOS_ERR,
 			       "%s: size mismatch in response to command %#x\n",
 			       __func__, command);
@@ -643,13 +499,11 @@
 		break;
 
 	case TPM2_GetCapability:
-		unmarshal_get_capability(&response_body, &cr_size,
-					 &tpm2_resp->gc);
+		rc |= unmarshal_get_capability(ib, &tpm2_resp->gc);
 		break;
 
 	case TPM2_NV_Read:
-		unmarshal_nv_read(&response_body, &cr_size,
-				  &tpm2_resp->nvr);
+		rc |= unmarshal_nv_read(ib, &tpm2_resp->nvr);
 		break;
 
 	case TPM2_Hierarchy_Control:
@@ -659,17 +513,18 @@
 	case TPM2_NV_WriteLock:
 	case TPM2_PCR_Extend:
 		/* Session data included in response can be safely ignored. */
-		cr_size = 0;
+		ibuf_oob_drain(ib, ibuf_remaining(ib));
 		break;
 
 	case TPM2_CR50_VENDOR_COMMAND:
-		unmarshal_vendor_command(&response_body, &cr_size,
-					 &tpm2_resp->vcr);
+		rc |= unmarshal_vendor_command(ib, &tpm2_resp->vcr);
 		break;
 
 	default:
 		{
-			int i;
+			size_t i;
+			size_t sz_left;
+			const uint8_t *data;
 
 			printk(BIOS_INFO, "%s:%d:"
 			       "Request to unmarshal unexpected command %#x,"
@@ -677,23 +532,25 @@
 			       __func__, __LINE__, command,
 			       tpm2_resp->hdr.tpm_code);
 
-			for (i = 0; i < cr_size; i++) {
+			sz_left = ibuf_remaining(ib);
+			data = ibuf_oob_drain(ib, sz_left);
+
+			for (i = 0; i < sz_left; i++) {
 				if (!(i % 16))
 					printk(BIOS_INFO, "\n");
-				printk(BIOS_INFO, "%2.2x ",
-				       ((uint8_t *)response_body)[i]);
+				printk(BIOS_INFO, "%2.2x ", data[i]);
 			}
 		}
 		printk(BIOS_INFO, "\n");
 		return NULL;
 	}
 
-	if (cr_size) {
+	if (ibuf_remaining(ib)) {
 		printk(BIOS_INFO,
 		       "%s:%d got %d bytes back in response to %#x,"
-		       " failed to parse (%d)\n",
+		       " failed to parse (%zd)\n",
 		       __func__, __LINE__, tpm2_resp->hdr.tpm_size,
-		       command, cr_size);
+		       command, ibuf_remaining(ib));
 		return NULL;
 	}
 
diff --git a/src/lib/tpm2_marshaling.h b/src/lib/tpm2_marshaling.h
index e177d06..5802044 100644
--- a/src/lib/tpm2_marshaling.h
+++ b/src/lib/tpm2_marshaling.h
@@ -6,6 +6,7 @@
 #ifndef __SRC_LIB_TPM2_MARSHALING_H
 #define __SRC_LIB_TPM2_MARSHALING_H
 
+#include <commonlib/iobuf.h>
 #include "tpm2_tlcl_structures.h"
 
 /* The below functions are used to serialize/deserialize TPM2 commands. */
@@ -18,14 +19,13 @@
  *
  * @command: code of the TPM2 command to marshal
  * @tpm_command_body: a pointer to the command specific structure
- * @buffer: buffer where command is marshaled to
- * @buffer_size: size of the buffer
+ * @ob: output buffer where command is marshaled to
  *
- * Returns number of bytes placed in the buffer, or -1 on error.
+ * Returns 0 on success or -1 on error.
  *
  */
 int tpm_marshal_command(TPM_CC command, void *tpm_command_body,
-			void *buffer, size_t buffer_size);
+			struct obuf *ob);
 
 /**
  * tpm_unmarshal_response
@@ -36,14 +36,11 @@
  * struct tpm2_response is a union of all possible responses.
  *
  * @command: code of the TPM2 command for which a response is unmarshaled
- * @response_body: buffer containing the serialized response.
- * @response_size: number of bytes in the buffer containing response
+ * @ib: input buffer containing the serialized response.
  *
  * Returns a pointer to the deserialized response or NULL in case of
  * unmarshaling problems.
  */
-struct tpm2_response *tpm_unmarshal_response(TPM_CC command,
-					     void *response_body,
-					     size_t response_size);
+struct tpm2_response *tpm_unmarshal_response(TPM_CC command, struct ibuf *ib);
 
 #endif // __SRC_LIB_TPM2_MARSHALING_H
diff --git a/src/lib/tpm2_tlcl.c b/src/lib/tpm2_tlcl.c
index 1118afb..754f835 100644
--- a/src/lib/tpm2_tlcl.c
+++ b/src/lib/tpm2_tlcl.c
@@ -23,29 +23,34 @@
 
 static void *tpm_process_command(TPM_CC command, void *command_body)
 {
-	ssize_t out_size;
+	struct obuf ob;
+	struct ibuf ib;
+	size_t out_size;
 	size_t in_size;
+	const uint8_t *sendb;
 	/* Command/response buffer. */
 	static uint8_t cr_buffer[TPM_BUFFER_SIZE] CAR_GLOBAL;
 
 	uint8_t *cr_buffer_ptr = car_get_var_ptr(cr_buffer);
 
-	out_size = tpm_marshal_command(command, command_body,
-				       cr_buffer_ptr, sizeof(cr_buffer));
-	if (out_size < 0) {
-		printk(BIOS_ERR, "command %#x, cr size %zd\n",
-		       command, out_size);
+	obuf_init(&ob, cr_buffer_ptr, sizeof(cr_buffer));
+
+	if (tpm_marshal_command(command, command_body, &ob) < 0) {
+		printk(BIOS_ERR, "command %#x\n", command);
 		return NULL;
 	}
 
+	sendb = obuf_contents(&ob, &out_size);
+
 	in_size = sizeof(cr_buffer);
-	if (tis_sendrecv(cr_buffer_ptr, out_size,
-			 cr_buffer_ptr, &in_size)) {
+	if (tis_sendrecv(sendb, out_size, cr_buffer_ptr, &in_size)) {
 		printk(BIOS_ERR, "tpm transaction failed\n");
 		return NULL;
 	}
 
-	return tpm_unmarshal_response(command, cr_buffer_ptr, in_size);
+	ibuf_init(&ib, cr_buffer_ptr, in_size);
+
+	return tpm_unmarshal_response(command, &ib);
 }
 
 

-- 
To view, visit https://review.coreboot.org/19063
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iec0bbff1312e8e6ec616d1528db8667f32e682c9
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)



More information about the coreboot-gerrit mailing list