The first patch in this series introduces a function for better handling the error paths of functions where commands to the TPM do not work as expected. It temporairly disables the device.
The other 4 patches refactor some of the code to get rid of local buffers and to make function easier to call. Some of the internal functions had to build up the same data structures as the BIOS interface expected, which was cumbersome.
Regards, Stefan
Stefan Berger (5): Temporarily deactivate the TPM in case of failure Refactor function building TPM commands Refactor the parameters being passed to tpm_extend_acpi_log Refactor hash_log_event BIOS interface function Refactor hash_log_extend_event
src/tcgbios.c | 273 ++++++++++++++++++++++++++++++++++------------------------ src/tcgbios.h | 3 +- 2 files changed, 163 insertions(+), 113 deletions(-)
From: Stefan Berger stefanb@linux.vnet.ibm.com
Temporarily deactivate the TPM in case of failure of TPM commands and failure to log measurements. Introduce the tpm_set_failure() function replacing occurrences of 'tpm_state.tpm_working = 0' and invoke it in error paths.
Temporarily deactivating the TPM means that it will be active again upon reboot.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/tcgbios.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------ src/tcgbios.h | 1 + 2 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 0995482..2f69318 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -64,6 +64,10 @@ static u8 evt_separator[] = {0xff,0xff,0xff,0xff};
/* local function prototypes */
+static u32 build_and_send_cmd(u8 locty, u32 ordinal, + const u8 *append, u32 append_size, + u8 *resbuffer, u32 return_size, u32 *returnCode, + enum tpmDurationType to_t); static u32 tpm_calling_int19h(void); static u32 tpm_add_event_separators(void); static u32 tpm_start_option_rom_scan(void); @@ -138,6 +142,31 @@ has_working_tpm(void) return tpm_state.tpm_working; }
+static void +tpm_set_failure(void) +{ + u32 returnCode; + + /* we will try to deactivate the TPM now - ignoring all errors */ + build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_CMD_ENABLE, + sizeof(PhysicalPresence_CMD_ENABLE), + NULL, 0, &returnCode, + TPM_DURATION_TYPE_SHORT); + + build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_PRESENT, + sizeof(PhysicalPresence_PRESENT), + NULL, 0, &returnCode, + TPM_DURATION_TYPE_SHORT); + + build_and_send_cmd(0, TPM_ORD_SetTempDeactivated, + NULL, 0, NULL, 0, &returnCode, + TPM_DURATION_TYPE_SHORT); + + tpm_state.tpm_working = 0; +} + static struct tcpa_descriptor_rev2 * find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp) { @@ -425,7 +454,7 @@ determine_timeouts(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); if (rc) return rc; return TCG_TCG_COMMAND_ERROR; @@ -495,7 +524,7 @@ tpm_startup(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); if (rc) return rc; return TCG_TCG_COMMAND_ERROR; @@ -555,7 +584,7 @@ tpm_prepboot(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); }
static int @@ -659,7 +688,7 @@ tpm_sha1_calc(const u8 *data, u32 length, u8 *hash) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM SHA1 malfunctioning.\n");
- tpm_state.tpm_working = 0; + tpm_set_failure(); if (rc) return rc; return TCG_TCG_COMMAND_ERROR; @@ -695,19 +724,28 @@ tpm_extend_acpi_log(void *entry_ptr, u16 *entry_count) u8 *log_area_start_address_next = NULL; struct pcpes *pcpes = (struct pcpes *)entry_ptr;
+ if (!has_working_tpm()) + return TCG_GENERAL_ERROR; + get_lasa_last_ptr(entry_count, &log_area_start_address_next);
dprintf(DEBUG_tcg, "TCGBIOS: LASA_BASE = %p, LASA_NEXT = %p\n", log_area_start_address_base, log_area_start_address_next);
- if (log_area_start_address_next == NULL || log_area_minimum_length == 0) + if (log_area_start_address_next == NULL || log_area_minimum_length == 0) { + tpm_set_failure(); + return TCG_PC_LOGOVERFLOW; + }
size = pcpes->eventdatasize + offsetof(struct pcpes, event);
if ((log_area_start_address_next + size - log_area_start_address_base) > log_area_minimum_length) { dprintf(DEBUG_tcg, "TCGBIOS: LOG OVERFLOW: size = %d\n", size); + + tpm_set_failure(); + return TCG_PC_LOGOVERFLOW; }
@@ -1476,5 +1514,5 @@ tpm_s3_resume(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); } diff --git a/src/tcgbios.h b/src/tcgbios.h index 4b7eaab..b0c20ad 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -56,6 +56,7 @@ #define TPM_ORD_PhysicalDisable 0x00000070 #define TPM_ORD_SetOwnerInstall 0x00000071 #define TPM_ORD_PhysicalSetDeactivated 0x00000072 +#define TPM_ORD_SetTempDeactivated 0x00000073 #define TPM_ORD_Startup 0x00000099 #define TPM_ORD_PhysicalPresence 0x4000000a #define TPM_ORD_Extend 0x00000014
On 11/12/2015 10:14 AM, Stefan Berger wrote:
Please disregard this patch. It's essentially the same as the 'other' 1/5.
Regards, Stefan
From: Stefan Berger stefanb@linux.vnet.ibm.com
Temporarily deactivate the TPM in case of failure of TPM commands and failure to log measurements. Introduce the tpm_set_failure() function replacing occurrences of 'tpm_state.tpm_working = 0' and invoke it in error paths.
Temporarily deactivating the TPM means that it will be active again upon reboot.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/tcgbios.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------ src/tcgbios.h | 1 + 2 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 0995482..2f69318 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -64,6 +64,10 @@ static u8 evt_separator[] = {0xff,0xff,0xff,0xff};
/* local function prototypes */
+static u32 build_and_send_cmd(u8 locty, u32 ordinal, + const u8 *append, u32 append_size, + u8 *resbuffer, u32 return_size, u32 *returnCode, + enum tpmDurationType to_t); static u32 tpm_calling_int19h(void); static u32 tpm_add_event_separators(void); static u32 tpm_start_option_rom_scan(void); @@ -138,6 +142,31 @@ has_working_tpm(void) return tpm_state.tpm_working; }
+static void +tpm_set_failure(void) +{ + u32 returnCode; + + /* we will try to deactivate the TPM now - ignoring all errors */ + build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_CMD_ENABLE, + sizeof(PhysicalPresence_CMD_ENABLE), + NULL, 0, &returnCode, + TPM_DURATION_TYPE_SHORT); + + build_and_send_cmd(0, TPM_ORD_PhysicalPresence, + PhysicalPresence_PRESENT, + sizeof(PhysicalPresence_PRESENT), + NULL, 0, &returnCode, + TPM_DURATION_TYPE_SHORT); + + build_and_send_cmd(0, TPM_ORD_SetTempDeactivated, + NULL, 0, NULL, 0, &returnCode, + TPM_DURATION_TYPE_SHORT); + + tpm_state.tpm_working = 0; +} + static struct tcpa_descriptor_rev2 * find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp) { @@ -425,7 +454,7 @@ determine_timeouts(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); if (rc) return rc; return TCG_TCG_COMMAND_ERROR; @@ -495,7 +524,7 @@ tpm_startup(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); if (rc) return rc; return TCG_TCG_COMMAND_ERROR; @@ -555,7 +584,7 @@ tpm_prepboot(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); }
static int @@ -659,7 +688,7 @@ tpm_sha1_calc(const u8 *data, u32 length, u8 *hash) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM SHA1 malfunctioning.\n");
- tpm_state.tpm_working = 0; + tpm_set_failure(); if (rc) return rc; return TCG_TCG_COMMAND_ERROR; @@ -695,19 +724,28 @@ tpm_extend_acpi_log(void *entry_ptr, u16 *entry_count) u8 *log_area_start_address_next = NULL; struct pcpes *pcpes = (struct pcpes *)entry_ptr;
+ if (!has_working_tpm()) + return TCG_GENERAL_ERROR; + get_lasa_last_ptr(entry_count, &log_area_start_address_next);
dprintf(DEBUG_tcg, "TCGBIOS: LASA_BASE = %p, LASA_NEXT = %p\n", log_area_start_address_base, log_area_start_address_next);
- if (log_area_start_address_next == NULL || log_area_minimum_length == 0) + if (log_area_start_address_next == NULL || log_area_minimum_length == 0) { + tpm_set_failure(); + return TCG_PC_LOGOVERFLOW; + }
size = pcpes->eventdatasize + offsetof(struct pcpes, event);
if ((log_area_start_address_next + size - log_area_start_address_base) > log_area_minimum_length) { dprintf(DEBUG_tcg, "TCGBIOS: LOG OVERFLOW: size = %d\n", size); + + tpm_set_failure(); + return TCG_PC_LOGOVERFLOW; }
@@ -1476,5 +1514,5 @@ tpm_s3_resume(void) err_exit: dprintf(DEBUG_tcg, "TCGBIOS: TPM malfunctioning (line %d).\n", __LINE__);
- tpm_state.tpm_working = 0; + tpm_set_failure(); } diff --git a/src/tcgbios.h b/src/tcgbios.h index 4b7eaab..b0c20ad 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -56,6 +56,7 @@ #define TPM_ORD_PhysicalDisable 0x00000070 #define TPM_ORD_SetOwnerInstall 0x00000071 #define TPM_ORD_PhysicalSetDeactivated 0x00000072 +#define TPM_ORD_SetTempDeactivated 0x00000073 #define TPM_ORD_Startup 0x00000099 #define TPM_ORD_PhysicalPresence 0x4000000a #define TPM_ORD_Extend 0x00000014
From: Stefan Berger stefanb@linux.vnet.ibm.com
Refactor the function building TPM commands to get rid of one of the buffers it uses for building a command. To do that, have it use the iovec also for the 'append' array that's being passed to the function.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/tcgbios.c | 45 +++++++++++++++++++-------------------------- src/tcgbios.h | 2 +- 2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 2f69318..3e9560b 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -330,47 +330,40 @@ build_and_send_cmd_od(u8 locty, u32 ordinal, const u8 *append, u32 append_size, const u8 *otherdata, u32 otherdata_size, enum tpmDurationType to_t) { -#define MAX_APPEND_SIZE sizeof(GetCapability_Timeouts) -#define MAX_RESPONSE_SIZE sizeof(struct tpm_res_getcap_perm_flags) u32 rc; - u8 ibuffer[TPM_REQ_HEADER_SIZE + MAX_APPEND_SIZE]; - u8 obuffer[MAX_RESPONSE_SIZE]; - struct tpm_req_header *trqh = (struct tpm_req_header *)ibuffer; + u8 obuffer[64]; + struct tpm_req_header trqh; struct tpm_rsp_header *trsh = (struct tpm_rsp_header *)obuffer; - struct iovec iovec[3]; + struct iovec iovec[4] = {{ 0 }}; u32 obuffer_len = sizeof(obuffer); u32 idx = 1;
- if (append_size > MAX_APPEND_SIZE || - return_size > MAX_RESPONSE_SIZE) { - dprintf(DEBUG_tcg, "TCGBIOS: size of requested buffers too big."); + if (return_size > sizeof(obuffer)) { + dprintf(DEBUG_tcg, "TCGBIOS: size of requested response too big."); return TCG_FIRMWARE_ERROR; }
- iovec[0].data = trqh; - iovec[0].length = TPM_REQ_HEADER_SIZE + append_size; + trqh.tag = cpu_to_be16(TPM_TAG_RQU_CMD); + trqh.totlen = cpu_to_be32(TPM_REQ_HEADER_SIZE + append_size + + otherdata_size); + trqh.ordinal = cpu_to_be32(ordinal);
- if (otherdata) { - iovec[1].data = (void *)otherdata; - iovec[1].length = otherdata_size; + iovec[0].data = &trqh; + iovec[0].length = TPM_REQ_HEADER_SIZE; + + if (append_size) { + iovec[1].data = append; + iovec[1].length = append_size; idx = 2; }
- iovec[idx].data = NULL; - iovec[idx].length = 0; + if (otherdata) { + iovec[idx].data = (void *)otherdata; + iovec[idx].length = otherdata_size; + }
- memset(ibuffer, 0x0, sizeof(ibuffer)); memset(obuffer, 0x0, sizeof(obuffer));
- trqh->tag = cpu_to_be16(TPM_TAG_RQU_CMD); - trqh->totlen = cpu_to_be32(TPM_REQ_HEADER_SIZE + append_size + - otherdata_size); - trqh->ordinal = cpu_to_be32(ordinal); - - if (append_size) - memcpy((char *)trqh + sizeof(*trqh), - append, append_size); - rc = transmit(locty, iovec, obuffer, &obuffer_len, to_t); if (rc) return rc; diff --git a/src/tcgbios.h b/src/tcgbios.h index b0c20ad..2b0b65d 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -108,7 +108,7 @@ enum irq_ids { struct iovec { size_t length; - void *data; + const void *data; };
From: Stefan Berger stefanb@linux.vnet.ibm.com
Refactor the parameters being passed to tpm_extend_acpi_log in such a way that the header of the logged event is passed in separate from the 'body'.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/tcgbios.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 3e9560b..5c44582 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -702,20 +702,23 @@ sha1_calc(const u8 *data, u32 length, u8 *hash) * Extend the ACPI log with the given entry by copying the * entry data into the log. * Input - * Pointer to the structure to be copied into the log + * pcpes : Pointer to the event 'header' to be copied into the log + * event : Pointer to the event 'body' to be copied into the log + * event_length: Length of the event array + * entry_count : optional pointer to get the current entry count * * Output: - * lower 16 bits of return code contain entry number - * if entry number is '0', then upper 16 bits contain error code. + * Returns an error code in case of faiure, 0 in case of success */ static u32 -tpm_extend_acpi_log(void *entry_ptr, u16 *entry_count) +tpm_extend_acpi_log(struct pcpes *pcpes, + const char *event, u32 event_length, + u16 *entry_count) { u32 log_area_minimum_length, size; u8 *log_area_start_address_base = get_lasa_base_ptr(&log_area_minimum_length); u8 *log_area_start_address_next = NULL; - struct pcpes *pcpes = (struct pcpes *)entry_ptr;
if (!has_working_tpm()) return TCG_GENERAL_ERROR; @@ -731,7 +734,7 @@ tpm_extend_acpi_log(void *entry_ptr, u16 *entry_count) return TCG_PC_LOGOVERFLOW; }
- size = pcpes->eventdatasize + offsetof(struct pcpes, event); + size = offsetof(struct pcpes, event) + event_length;
if ((log_area_start_address_next + size - log_area_start_address_base) > log_area_minimum_length) { @@ -742,9 +745,14 @@ tpm_extend_acpi_log(void *entry_ptr, u16 *entry_count) return TCG_PC_LOGOVERFLOW; }
- memcpy(log_area_start_address_next, entry_ptr, size); + pcpes->eventdatasize = event_length;
- (*entry_count)++; + memcpy(log_area_start_address_next, pcpes, offsetof(struct pcpes, event)); + memcpy(log_area_start_address_next + offsetof(struct pcpes, event), + event, event_length); + + if (entry_count) + (*entry_count)++;
return 0; } @@ -920,7 +928,9 @@ hash_log_event(const struct hlei *hlei, struct hleo *hleo) return rc; }
- rc = tpm_extend_acpi_log((void *)hlei->logdataptr, &entry_count); + rc = tpm_extend_acpi_log(pcpes, + (char *)&pcpes->event, pcpes->eventdatasize, + &entry_count); if (rc) goto err_exit;
From: Stefan Berger stefanb@linux.vnet.ibm.com
Refactor the signature of hash_log_event to take individual pointers as parameters and introduce hash_log_event_int as an function to be called with the parameters passed from the BIOS interrupt.
Refactor existing callers to hash_log_event that now do not have to build up the data structures expected by the BIOS interface.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/tcgbios.c | 57 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 5c44582..9a17b33 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -892,9 +892,28 @@ hash_all(const struct hai *hai, u8 *hash) return sha1_calc((const u8 *)hai->hashdataptr, hai->hashdatalen, hash); }
+static u32 +hash_log_event(const void *hashdata, u32 hashdata_length, + struct pcpes *pcpes, + const char *event, u32 event_length, + u16 *entry_count) +{ + u32 rc = 0; + + if (pcpes->pcrindex >= 24) + return TCG_INVALID_INPUT_PARA; + + if (hashdata) { + rc = sha1_calc(hashdata, hashdata_length, pcpes->digest); + if (rc) + return rc; + } + + return tpm_extend_acpi_log(pcpes, event, event_length, entry_count); +}
static u32 -hash_log_event(const struct hlei *hlei, struct hleo *hleo) +hash_log_event_int(const struct hlei *hlei, struct hleo *hleo) { u32 rc = 0; u16 size; @@ -916,21 +935,16 @@ hash_log_event(const struct hlei *hlei, struct hleo *hleo)
if (pcpes->pcrindex >= 24 || pcpes->pcrindex != hlei->pcrindex || - pcpes->eventtype != hlei->logeventtype) { + pcpes->eventtype != hlei->logeventtype || + hlei->logdatalen != + offsetof(struct pcpes, event) + pcpes->eventdatasize) { rc = TCG_INVALID_INPUT_PARA; goto err_exit; }
- if ((hlei->hashdataptr != 0) && (hlei->hashdatalen != 0)) { - rc = sha1_calc((const u8 *)hlei->hashdataptr, - hlei->hashdatalen, pcpes->digest); - if (rc) - return rc; - } - - rc = tpm_extend_acpi_log(pcpes, - (char *)&pcpes->event, pcpes->eventdatasize, - &entry_count); + rc = hash_log_event(hlei->hashdataptr, hlei->hashdatalen, + pcpes, (char *)&pcpes->event, pcpes->eventdatasize, + &entry_count); if (rc) goto err_exit;
@@ -980,18 +994,11 @@ hash_log_extend_event(const struct hleei_short *hleei_s, struct hleeo *hleeo) }
pcpes = (struct pcpes *)logdataptr; + (void)logdatalen; /* only temporary */
- struct hlei hlei = { - .ipblength = sizeof(hlei), - .hashdataptr = hleei_s->hashdataptr, - .hashdatalen = hleei_s->hashdatalen, - .pcrindex = hleei_s->pcrindex, - .logeventtype= pcpes->eventtype, - .logdataptr = logdataptr, - .logdatalen = logdatalen, - }; - - rc = hash_log_event(&hlei, &hleo); + rc = hash_log_event(hleei_s->hashdataptr, hleei_s->hashdatalen, + pcpes, (char *)&pcpes->event, pcpes->eventdatasize, + NULL); if (rc) goto err_exit;
@@ -1110,8 +1117,8 @@ tpm_interrupt_handler32(struct bregs *regs) break;
case TCG_HashLogEvent: - regs->eax = hash_log_event((struct hlei*)input_buf32(regs), - (struct hleo*)output_buf32(regs)); + regs->eax = hash_log_event_int((struct hlei*)input_buf32(regs), + (struct hleo*)output_buf32(regs)); break;
case TCG_HashAll:
From: Stefan Berger stefanb@linux.vnet.ibm.com
Refactor the signature of the hash_log_extend_event to take individual pointers as parameters and introduce hash_log_extend_event_int as a function to be called with the parameters passed from the BIOS interrupt.
Refactor existing callers to hash_log_extend_event that now do not have to build up the data structure expected by the BIOS interface.
Signed-off-by: Stefan Berger stefanb@linux.vnet.ibm.com --- src/tcgbios.c | 109 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/src/tcgbios.c b/src/tcgbios.c index 9a17b33..c4e3b5e 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -962,9 +962,25 @@ err_exit: return rc; }
+static u32 +hash_log_extend_event(const void *hashdata, u32 hashdata_length, + struct pcpes *pcpes, + const char *event, u32 event_length, + u32 pcrindex, u16 *entry_count) +{ + u32 rc; + + rc = hash_log_event(hashdata, hashdata_length, pcpes, + event, event_length, entry_count); + if (rc) + return rc; + + return tpm_extend(pcpes->digest, pcrindex); +}
static u32 -hash_log_extend_event(const struct hleei_short *hleei_s, struct hleeo *hleeo) +hash_log_extend_event_int(const struct hleei_short *hleei_s, + struct hleeo *hleeo) { u32 rc = 0; struct hleo hleo; @@ -972,6 +988,7 @@ hash_log_extend_event(const struct hleei_short *hleei_s, struct hleeo *hleeo) const void *logdataptr; u32 logdatalen; struct pcpes *pcpes; + u32 pcrindex;
/* short or long version? */ switch (hleei_s->ipblength) { @@ -979,12 +996,14 @@ hash_log_extend_event(const struct hleei_short *hleei_s, struct hleeo *hleeo) /* short */ logdataptr = hleei_s->logdataptr; logdatalen = hleei_s->logdatalen; + pcrindex = hleei_s->pcrindex; break;
case sizeof(struct hleei_long): /* long */ logdataptr = hleei_l->logdataptr; logdatalen = hleei_l->logdatalen; + pcrindex = hleei_l->pcrindex; break;
default: @@ -994,11 +1013,18 @@ hash_log_extend_event(const struct hleei_short *hleei_s, struct hleeo *hleeo) }
pcpes = (struct pcpes *)logdataptr; - (void)logdatalen; /* only temporary */
- rc = hash_log_event(hleei_s->hashdataptr, hleei_s->hashdatalen, - pcpes, (char *)&pcpes->event, pcpes->eventdatasize, - NULL); + if (pcpes->pcrindex >= 24 || + pcpes->pcrindex != pcrindex || + logdatalen != offsetof(struct pcpes, event) + pcpes->eventdatasize) { + rc = TCG_INVALID_INPUT_PARA; + goto err_exit; + } + + rc = hash_log_extend_event(hleei_s->hashdataptr, hleei_s->hashdatalen, + pcpes, + (char *)&pcpes->event, pcpes->eventdatasize, + pcrindex, NULL); if (rc) goto err_exit;
@@ -1006,8 +1032,6 @@ hash_log_extend_event(const struct hleei_short *hleei_s, struct hleeo *hleeo) hleeo->reserved = 0; hleeo->eventnumber = hleo.eventnumber;
- rc = tpm_extend(pcpes->digest, hleei_s->pcrindex); - err_exit: if (rc != 0) { hleeo->opblength = 4; @@ -1045,25 +1069,21 @@ compact_hash_log_extend_event(u8 *buffer, u32 *edx_ptr) { u32 rc = 0; - struct hleeo hleeo; struct pcpes pcpes = { .pcrindex = pcrindex, .eventtype = EV_COMPACT_HASH, .eventdatasize = sizeof(info), .event = info, }; - struct hleei_short hleei = { - .ipblength = sizeof(hleei), - .hashdataptr = buffer, - .hashdatalen = length, - .pcrindex = pcrindex, - .logdataptr = &pcpes, - .logdatalen = sizeof(pcpes), - }; + u16 entry_count; + + rc = hash_log_extend_event(buffer, length, + &pcpes, + (char *)&pcpes.event, pcpes.eventdatasize, + pcpes.pcrindex, &entry_count);
- rc = hash_log_extend_event(&hleei, &hleeo); if (rc == 0) - *edx_ptr = hleeo.eventnumber; + *edx_ptr = entry_count;
return rc; } @@ -1101,7 +1121,7 @@ tpm_interrupt_handler32(struct bregs *regs)
case TCG_HashLogExtendEvent: regs->eax = - hash_log_extend_event( + hash_log_extend_event_int( (struct hleei_short *)input_buf32(regs), (struct hleeo *)output_buf32(regs)); break; @@ -1153,46 +1173,27 @@ tpm_interrupt_handler32(struct bregs *regs) * appended to the TCG_PCClientPCREventStruct * * Input parameters: - * pcrIndex : which PCR to extend + * pcrindex : which PCR to extend * event_type : type of event; specs section on 'Event Types' - * info : pointer to info (e.g., string) to be added to log as-is - * info_length: length of the info - * data : pointer to the data (i.e., string) to be added to the log - * data_length: length of the data + * event : pointer to info (e.g., string) to be added to log as-is + * event_length: length of the event + * hashdata : pointer to the data to be hashed + * hashdata_length: length of the data to be hashed */ static u32 -tpm_add_measurement_to_log(u32 pcrIndex, u32 event_type, - const char *info, u32 info_length, - const u8 *data, u32 data_length) +tpm_add_measurement_to_log(u32 pcrindex, u32 event_type, + const char *event, u32 event_length, + const u8 *hashdata, u32 hashdata_length) { - u32 rc = 0; - struct hleeo hleeo; - u8 _pcpes[offsetof(struct pcpes, event) + 400]; - struct pcpes *pcpes = (struct pcpes *)_pcpes; - - if (info_length < sizeof(_pcpes) - offsetof(struct pcpes, event)) { - - pcpes->pcrindex = pcrIndex; - pcpes->eventtype = event_type; - memset(&pcpes->digest, 0x0, sizeof(pcpes->digest)); - pcpes->eventdatasize = info_length; - memcpy(&pcpes->event, info, info_length); - - struct hleei_short hleei = { - .ipblength = sizeof(hleei), - .hashdataptr = data, - .hashdatalen = data_length, - .pcrindex = pcrIndex, - .logdataptr = _pcpes, - .logdatalen = info_length + offsetof(struct pcpes, event), - }; - - rc = hash_log_extend_event(&hleei, &hleeo); - } else { - rc = TCG_GENERAL_ERROR; - } + struct pcpes pcpes = { + .pcrindex = pcrindex, + .eventtype = event_type, + }; + u16 entry_count;
- return rc; + return hash_log_extend_event(hashdata, hashdata_length, &pcpes, + event, event_length, pcrindex, + &entry_count); }
On Thu, Nov 12, 2015 at 10:14:43AM -0500, Stefan Berger wrote:
The first patch in this series introduces a function for better handling the error paths of functions where commands to the TPM do not work as expected. It temporairly disables the device.
The other 4 patches refactor some of the code to get rid of local buffers and to make function easier to call. Some of the internal functions had to build up the same data structures as the BIOS interface expected, which was cumbersome.
Thanks. The series looks good to me. I think this should be applied after the 1.9.0 release though.
-Kevin
"Kevin O'Connor" kevin@koconnor.net wrote on 11/17/2015 08:50:43 AM:
From: "Kevin O'Connor" kevin@koconnor.net To: Stefan Berger/Watson/IBM@IBMUS Cc: seabios@seabios.org Date: 11/17/2015 08:51 AM Subject: Re: [PATCH 0/5] Improve TPM related code
On Thu, Nov 12, 2015 at 10:14:43AM -0500, Stefan Berger wrote:
The first patch in this series introduces a function for better
handling
the error paths of functions where commands to the TPM do not work as expected. It temporairly disables the device.
The other 4 patches refactor some of the code to get rid of local
buffers
and to make function easier to call. Some of the internal functions
had
to build up the same data structures as the BIOS interface expected,
which
was cumbersome.
Thanks. The series looks good to me. I think this should be applied after the 1.9.0 release though.
For a 1.9.1 maybe :-) Thanks.
Stefan
On Thu, Nov 12, 2015 at 10:14:43AM -0500, Stefan Berger wrote:
The first patch in this series introduces a function for better handling the error paths of functions where commands to the TPM do not work as expected. It temporairly disables the device.
The other 4 patches refactor some of the code to get rid of local buffers and to make function easier to call. Some of the internal functions had to build up the same data structures as the BIOS interface expected, which was cumbersome.
Thanks - I made some minor changes to the commit messages and committed this series along with your follow up gcc34 patch. (I added a "tpm: " prefix and clarified that the gcc34 issue was a warning.)
-Kevin
-
Kevin O'Connor -
Stefan Berger -
Stefan Berger