[SLOF] [PATCH v2 05/20] Extend internal firmware API

[Thomas Huth]

On 17/11/15 18:02, Stefan Berger wrote:
> From: Stefan Berger <stefanb at linux.vnet.ibm.com>
> 
> Extend the internal API of the TPM firmware support with additional
> functions for hashing data, extending the TPM's platform configuration
> registers with a hash, and appending to the log that is recording
> what was hashed.
> 
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> ---
>  lib/libtpm/tcgbios.c     | 285 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/libtpm/tcgbios_int.h |   2 +
>  2 files changed, 287 insertions(+)
> 
> diff --git a/lib/libtpm/tcgbios.c b/lib/libtpm/tcgbios.c
> index 7e2ae1e..e22c550 100644
> --- a/lib/libtpm/tcgbios.c
> +++ b/lib/libtpm/tcgbios.c
> @@ -25,6 +25,9 @@
>  #include "tcgbios.h"
>  #include "tcgbios_int.h"
>  #include "stdio.h"
> +#include "sha1.h"
> +#include "stddef.h"

Please use <stddef.h> for standard includes.

> +#include "helpers.h"
>  
>  #undef TCGBIOS_DEBUG
>  //#define TCGBIOS_DEBUG
> @@ -82,6 +85,8 @@ extern struct tpm_driver tpm_drivers[];
>  
>  static void *log_base;
>  static uint32_t log_area_size;
> +/* next log entry goes here */
> +static void *log_area_address_next;
>  
>  /* prototypes */
>  static uint32_t build_and_send_cmd(uint32_t ordinal, const uint8_t *append,
> @@ -89,6 +94,8 @@ static uint32_t build_and_send_cmd(uint32_t ordinal, const uint8_t *append,
>  				   uint32_t return_size, uint32_t *return_code,
>  				   enum tpm_duration_type to_t);
>  
> +static bool has_working_tpm(void);
> +
>  /********************************************************
>    Extensions for TCG-enabled BIOS
>   *******************************************************/
> @@ -98,6 +105,7 @@ void tpm_set_log_parameters(void *addr, unsigned int size)
>  	dprintf("Log is at 0x%llx; size is %u bytes\n",
>  		(uint64_t)addr, size);
>  	log_base = addr;
> +	log_area_address_next = addr;
>  	log_area_size = size;
>  }
>  
> @@ -120,6 +128,66 @@ static void reset_ofdt_log(void)
>  		memset(log_area_start_address, 0, get_log_area_size());
>  }
>  
> +static void set_log_area_address_next(void *next)
> +{
> +	log_area_address_next = next;
> +}
> +
> +static void *get_log_area_address_next(void)
> +{
> +	return log_area_address_next;
> +}
> +
> +/*
> + * Extend the ACPI log with the given entry by copying the
> + * entry data into the log.

ACPI log? There's no ACPI on POWER as far as I know.

> + * @pcpes: Pointer to the structure to be copied into the log
> + * @event: The event to be appended to 'pcpes'
> + * @event_length: The length of the event
> + *
> + * Output:
> + *  lower 16 bits of return code contain entry number
> + *  if entry number is '0', then upper 16 bits contain error code.
> + */
> +static uint32_t tpm_extend_ofdt_log(struct pcpes *pcpes,
> +				    const char *event, uint32_t event_length)
> +{
> +	uint32_t size;
> +	uint32_t log_area_size = get_log_area_size();
> +	uint8_t *log_area_start_address = get_log_base_ptr();

Uh, so you've now got local variables with the same name as the global
static variables that contain the same information as the global static
variables ... well, IMHO please use the global static variables directly
instead.

> +	uint8_t *log_area_address_next = get_log_area_address_next();
> +
> +	if (!has_working_tpm()) {
> +		dprintf("Not appending to log due to TPM error state\n");
> +		return TCGBIOS_FATAL_COM_ERROR;
> +	}
> +
> +	dprintf("LASA_BASE = %p, LASA_NEXT = %p\n",
> +		log_area_start_address, log_area_address_next);
> +
> +	if (!log_area_address_next || log_area_size == 0)
> +		return TCGBIOS_LOGOVERFLOW;
> +
> +	size = offset_of(struct pcpes, event) + event_length;
> +
> +	if ((log_area_address_next + size - log_area_start_address) >
> +	     log_area_size) {
> +		dprintf("LOG OVERFLOW: size = %d\n", size);
> +		return TCGBIOS_LOGOVERFLOW;
> +	}
> +
> +	pcpes->eventdatasize = event_length;
> +
> +	memcpy(log_area_address_next, pcpes, offset_of(struct pcpes, event));
> +	memcpy(log_area_address_next + offset_of(struct pcpes, event),
> +	       event, event_length);
> +
> +	set_log_area_address_next(log_area_address_next + size);
> +
> +	return 0;
> +}

[...]

> +/*
> + * Extend a PCR of the TPM with the given hash
> + *
> + * @hash: sha1 hash (20 bytes) to extend PCR with
> + * @pcrindex: the PCR to extend [ 0..23 ]
> + */
> +static uint32_t tpm_extend(uint8_t *hash, uint32_t pcrindex)
> +{
> +	struct tpm_req_extend req = {
> +		.tag	  = cpu_to_be16(TPM_TAG_RQU_CMD),
> +		.totlen   = cpu_to_be32(sizeof(req)),
> +		.ordinal  = cpu_to_be32(TPM_ORD_EXTEND),
> +		.pcrindex = cpu_to_be32(pcrindex),
> +	};
> +	struct tpm_rsp_extend rsp;
> +	uint32_t rsplen = sizeof(rsp);
> +	uint32_t rc;
> +
> +	memcpy(req.digest, hash, sizeof(req.digest));
> +
> +	rc = pass_through_to_tpm((unsigned char *)&req, sizeof(req),
> +				 TPM_DURATION_TYPE_SHORT,
> +				 (unsigned char *)&rsp, &rsplen);
> +	if (!rc) {
> +		if (rsplen != sizeof(rsp)) {
> +			dprintf("TPM_Extend response has unexpected size: %u\n",
> +				rsplen);
> +			rc = TCGBIOS_FATAL_COM_ERROR;
> +		}
> +	}
> +
> +	if (rc)

you could also use "else" here.

> +		tpm_shutdown();
> +
> +	return rc;
> +}

 Thomas