[SeaBIOS] [PATCH 2/2] tpm: Write logs in TPM 2 format

[Kevin O'Connor]

On Mon, Feb 08, 2016 at 07:25:35AM -0500, Stefan Berger wrote:
> On 02/06/2016 01:35 PM, Kevin O'Connor wrote:
> >Add support for the TPM 2 format of log messages.
> >
> >Write the logs in the format that is appropriate for the version of
> >the host's TPM. For TPM 1.2 write it in the 'pcpes' structure's
> >format, for TPM 2 in the new TPM 2 format.
> >
> >By using this method we can keep the API interface on systems with a
> >TPM 2 even though applications pass in the 'pcpes' structures
> >directly. The log will still be written in the appropriate format.
> >
> >The TPM 2 log contains a TPM 1.2 type of entry of event type
> >EV_NO_ACTION and entry of type TCG_EfiSpeIdEventStruct as the first
> >entry. This is described in the EFI specification (section 5.3):
> >
> >Signed-off-by: Kevin O'Connor <kevin at koconnor.net>
> >---
> >  src/std/tcg.h | 35 +++++++++++++++++++++++++++
> >  src/tcgbios.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
> >  2 files changed, 101 insertions(+), 10 deletions(-)
> >
> >diff --git a/src/std/tcg.h b/src/std/tcg.h
> >index dbb3a60..c59f671 100644
> >--- a/src/std/tcg.h
> >+++ b/src/std/tcg.h
> >@@ -91,6 +91,7 @@ enum irq_ids {
> >
> >  /* event types: 10.4.1 / table 11 */
> >  #define EV_POST_CODE             1
> >+#define EV_NO_ACTION             3
> >  #define EV_SEPARATOR             4
> >  #define EV_ACTION                5
> >  #define EV_EVENT_TAG             6
> >@@ -474,4 +475,38 @@ struct tpm2_req_hierarchycontrol {
> >      u8 state;
> >  } PACKED;
> >
> >+/* TPM 2 log entry */
> >+
> >+struct tpml_digest_values_sha1 {
> >+    u16 hashtype;
> >+    u8 sha1[SHA1_BUFSIZE];
> >+};
> >+
> >+struct tcg_pcr_event2_sha1 {
> >+    u32 pcrindex;
> >+    u32 eventtype;
> >+    u32 count; /* number of digests */
> >+    struct tpml_digest_values_sha1 digests[1];
> >+    u32 eventdatasize;
> >+    u8 event[0];
> >+} PACKED;
> >+
> >+struct TCG_EfiSpecIdEventStruct {
> >+    u8 signature[16];
> >+    u32 platformClass;
> >+    u8 specVersionMinor;
> >+    u8 specVersionMajor;
> >+    u8 specErrata;
> >+    u8 uintnSize;
> >+    u32 numberOfAlgorithms;
> >+    struct TCG_EfiSpecIdEventAlgorithmSize {
> >+        u16 algorithmId;
> >+        u16 digestSize;
> >+    } digestSizes[1];
> >+    u8 vendorInfoSize;
> >+    u8 vendorInfo[0];
> >+};
> >+
> >+#define TPM_TCPA_ACPI_CLASS_CLIENT 0
> >+
> >  #endif // tcg.h
> >diff --git a/src/tcgbios.c b/src/tcgbios.c
> >index cddc99b..f0ab188 100644
> >--- a/src/tcgbios.c
> >+++ b/src/tcgbios.c
> >@@ -141,7 +141,7 @@ tpm_tcpa_probe(void)
> >   *  Returns an error code in case of faiure, 0 in case of success
> >   */
> >  static int
> >-tpm_log_event(struct pcpes *pcpes, const void *event)
> >+tpm_log_event(struct tcg_pcr_event2_sha1 *entry, const void *event)
> >  {
> >      dprintf(DEBUG_tcg, "TCGBIOS: LASA = %p, next entry = %p\n",
> >              tpm_state.log_area_start_address, tpm_state.log_area_next_entry);
> >@@ -149,7 +149,7 @@ tpm_log_event(struct pcpes *pcpes, const void *event)
> >      if (tpm_state.log_area_next_entry == NULL)
> >          return -1;
> >
> >-    u32 size = sizeof(*pcpes) + pcpes->eventdatasize;
> >+    u32 size = sizeof(*entry) + entry->eventdatasize;
> 
> This is only the correct size for TPM 2. I think there should be a case
> statement here.

I didn't think it would matter to overestimate a v1 record size by 6
bytes.  That is, the acpi log shouldn't overflow, and if it gets that
close to overflowing then one could just call it overflowed?

> >
> >      if ((tpm_state.log_area_next_entry + size - tpm_state.log_area_start_address) >
> >           tpm_state.log_area_minimum_length) {
> >@@ -157,9 +157,19 @@ tpm_log_event(struct pcpes *pcpes, const void *event)
> >          return -1;
> >      }
> >
> >-    memcpy(tpm_state.log_area_next_entry, pcpes, sizeof(*pcpes));
> >-    memcpy(tpm_state.log_area_next_entry + sizeof(*pcpes),
> >-           event, pcpes->eventdatasize);
> >+    if (TPM_version == TPM_VERSION_1_2 || entry->eventtype == EV_NO_ACTION) {
> 
> This only works if TPM 2 entries don't have a EV_NO_ACTION (= 3). I think
> the cleaner solution would be to differentiate by version only.

Okay.

> The main difference here between parts of my code and yours is that you are
> inlining it.

Yes.  Avoiding 'struct log_entry' was the main thing though.

Thanks,
-Kevin