Re: [SeaBIOS] [PATCH 4/8] tpm: Use pcpes->event[] to pass data to tpm_extend_acpi_log()

[Stefan Berger]

On 11/22/2015 08:02 PM, Kevin O'Connor wrote:
> Instead of passing in the event data to tpm_extend_acpi_log() via
> parameters, use the event[] field in the pcpes.  Update those callers
> that don't populate the pcpes->event to do so prior to calling
> tpm_extend_acpi_log().
> Signed-off-by: Kevin O'Connor
>
> src/std/tcg.h |  2 +-
>   src/tcgbios.c | 76 +++++++++++++++++++++++++++++------------------------------
>   2 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/src/std/tcg.h b/src/std/tcg.h
> index 88b2688..5f9cad9 100644
> --- a/src/std/tcg.h
> +++ b/src/std/tcg.h
> @@ -207,7 +207,7 @@ struct pcpes
>       u32    eventtype;
>       u8     digest[SHA1_BUFSIZE];
>       u32    eventdatasize;
> u32    event;
>
> u8     event[0];
> } PACKED;
>
> struct pcctes
>
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 0c28935..e7adf3f 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -290,18 +290,15 @@ reset_acpi_log(void)
> Returns an error code in case of faiure, 0 in case of success
>
> */
>   static u32
> -tpm_extend_acpi_log(struct pcpes *pcpes,
>                const char *event, u32 event_length)
>
> +tpm_extend_acpi_log(struct pcpes *pcpes)
>   {
>
> u32 size;
> dprintf(DEBUG_tcg, "TCGBIOS: LASA = %p, next entry = %p\n",
>         tpm_state.log_area_start_address, tpm_state.log_area_next_entry);
>
> if (tpm_state.log_area_next_entry == NULL)
>     return TCG_PC_LOGOVERFLOW;
>
> size = offsetof(struct pcpes, event) + event_length;
>
> u32 size = sizeof(*pcpes) + pcpes->eventdatasize;
>
> if ((tpm_state.log_area_next_entry + size - tpm_state.log_area_start_address) >
>       tpm_state.log_area_minimum_length) {
>
> @@ -309,11 +306,7 @@ tpm_extend_acpi_log(struct pcpes *pcpes,
>           return TCG_PC_LOGOVERFLOW;
>       }
> pcpes->eventdatasize = event_length;
> memcpy(tpm_state.log_area_next_entry, pcpes, offsetof(struct pcpes, event));
> memcpy(tpm_state.log_area_next_entry + offsetof(struct pcpes, event),
>       event, event_length);
>
> memcpy(tpm_state.log_area_next_entry, pcpes, size);
>
> tpm_state.log_area_last_entry = tpm_state.log_area_next_entry;
>  tpm_state.log_area_next_entry += size;
>
> @@ -513,7 +506,7 @@ tpm_extend(u8 *hash, u32 pcrindex)
>   }
>
> static u32
> -tpm_log_event(struct pcpes *pcpes, const char *event, u32 event_length)
> +tpm_log_event(struct pcpes *pcpes)
>   {
>       if (pcpes->pcrindex >= 24)
>           return TCG_INVALID_INPUT_PARA;
> @@ -521,16 +514,16 @@ tpm_log_event(struct pcpes *pcpes, const char *event, u32 event_length)
>       if (!has_working_tpm())
>           return TCG_GENERAL_ERROR;
> u32 rc = tpm_extend_acpi_log(pcpes, event, event_length);
>
> u32 rc = tpm_extend_acpi_log(pcpes);
>  if (rc)
>      tpm_set_failure();
>  return rc;
> }
>
> static u32
>
> -tpm_log_extend_event(struct pcpes *pcpes, const char *event, u32 event_length)
> +tpm_log_extend_event(struct pcpes *pcpes)
>   {
> u32 rc = tpm_log_event(pcpes, event, event_length);
> u32 rc = tpm_log_event(pcpes);
>  if (rc)
>      return rc;
>  return tpm_extend(pcpes->digest, pcpes->pcrindex);
>
> @@ -560,12 +553,21 @@ tpm_add_measurement_to_log(u32 pcrindex, u32 event_type,
>                              const char *event, u32 event_length,
>                              const u8 *hashdata, u32 hashdata_length)
>   {
> struct pcpes pcpes = {
>    .pcrindex = pcrindex,
>    .eventtype = event_type,
> struct {
>    struct pcpes pcpes;
>    u8 event[128];

You are undoing a previous change now. I had tried to prevent having 
this event buffer (on the stack) but allow higher layers to pass 
arbitrary-length event buffers around that get copied into the log at 
the very end. This was to avoid having to append them here.

Stefan