[PATCH] efi_loader: Fix spec ID event creation

[Ilias Apalodimas]

On Tue, Sep 14, 2021 at 12:14:31PM +0530, Ruchika Gupta wrote:
> TCG EFI Protocol Specification defines the number_of_algorithms
> field in spec ID event to be equal to the number of active
> algorithms supported by the TPM device. In current implementation,
> this field is populated with the count of all algorithms supported
> by the TPM which leads to incorrect spec ID event creation.
> 
> Similarly, the algorithm array in spec ID event should be a variable
> length array with length being equal to the number_of_algorithms field.
> In current implementation this is defined as a fixed length array
> which has been fixed.
> 
> Signed-off-by: Ruchika Gupta <ruchika.gupta at linaro.org>
> CC: Masahisa Kojima <masahisa.kojima at linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> CC: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  include/efi_tcg2.h        |  7 +------
>  lib/efi_loader/efi_tcg2.c | 40 ++++++++++++++++++++++-----------------
>  2 files changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index c99384fb00..6c9f448a26 100644
> --- a/include/efi_tcg2.h
> +++ b/include/efi_tcg2.h
> @@ -165,8 +165,6 @@ struct tcg_efi_spec_id_event_algorithm_size {
>   * @digest_sizes:		array of number_of_algorithms pairs
>   *				1st member defines the algorithm id
>   *				2nd member defines the algorithm size
> - * @vendor_info_size:		size in bytes for vendor specific info
> - * @vendor_info:		vendor specific info
>   */
>  struct tcg_efi_spec_id_event {
>  	u8 signature[16];
> @@ -176,10 +174,7 @@ struct tcg_efi_spec_id_event {
>  	u8 spec_errata;
>  	u8 uintn_size;
>  	u32 number_of_algorithms;
> -	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[TPM2_NUM_PCR_BANKS];
> -	u8 vendor_info_size;
> -	/* U-Boot does not provide any vendor info */
> -	u8 vendor_info[];
> +	struct tcg_efi_spec_id_event_algorithm_size digest_sizes[];
>  } __packed;
>  
>  /**
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index b268a02976..3fd6bc30a1 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -575,9 +575,10 @@ static efi_status_t tcg2_create_digest(const u8 *input, u32 length,
>  			EFI_PRINT("Unsupported algorithm %x\n", hash_alg);
>  			return EFI_INVALID_PARAMETER;
>  		}
> +		digest_list->digests[digest_list->count].hash_alg = hash_alg;
> +		memcpy(&digest_list->digests[digest_list->count].digest, final,
> +		       (u32)alg_to_len(hash_alg));
>  		digest_list->count++;
> -		digest_list->digests[i].hash_alg = hash_alg;
> -		memcpy(&digest_list->digests[i].digest, final, (u32)alg_to_len(hash_alg));
>  	}
>  
>  	return EFI_SUCCESS;
> @@ -798,8 +799,9 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size,
>  			EFI_PRINT("Unsupported algorithm %x\n", hash_alg);
>  			return EFI_INVALID_PARAMETER;
>  		}
> -		digest_list->digests[i].hash_alg = hash_alg;
> -		memcpy(&digest_list->digests[i].digest, hash, (u32)alg_to_len(hash_alg));
> +		digest_list->digests[digest_list->count].hash_alg = hash_alg;
> +		memcpy(&digest_list->digests[digest_list->count].digest, hash,
> +		       (u32)alg_to_len(hash_alg));
>  		digest_list->count++;
>  	}
>  
> @@ -1123,7 +1125,7 @@ static efi_status_t create_specid_event(struct udevice *dev, void *buffer,
>  	struct tcg_efi_spec_id_event *spec_event;
>  	size_t spec_event_size;
>  	efi_status_t ret = EFI_DEVICE_ERROR;
> -	u32 active = 0, supported = 0;
> +	u32 active = 0, supported = 0, pcr_count = 0, alg_count = 0;
>  	int err;
>  	size_t i;
>  
> @@ -1145,25 +1147,29 @@ static efi_status_t create_specid_event(struct udevice *dev, void *buffer,
>  		TCG_EFI_SPEC_ID_EVENT_SPEC_VERSION_ERRATA_TPM2;
>  	spec_event->uintn_size = sizeof(efi_uintn_t) / sizeof(u32);
>  
> -	err = tpm2_get_pcr_info(dev, &supported, &active,
> -				&spec_event->number_of_algorithms);
> +	err = tpm2_get_pcr_info(dev, &supported, &active, &pcr_count);
> +
>  	if (err)
>  		goto out;
> -	if (spec_event->number_of_algorithms > MAX_HASH_COUNT ||
> -	    spec_event->number_of_algorithms < 1)
> -		goto out;
>  
> -	for (i = 0; i < spec_event->number_of_algorithms; i++) {
> +	for (i = 0; i < pcr_count; i++) {
>  		u16 hash_alg = hash_algo_list[i].hash_alg;
>  		u16 hash_len = hash_algo_list[i].hash_len;
>  
> -		if (active && alg_to_mask(hash_alg)) {
> +		if (active & alg_to_mask(hash_alg)) {
>  			put_unaligned_le16(hash_alg,
> -					   &spec_event->digest_sizes[i].algorithm_id);
> +					   &spec_event->digest_sizes[alg_count].algorithm_id);
>  			put_unaligned_le16(hash_len,
> -					   &spec_event->digest_sizes[i].digest_size);
> +					   &spec_event->digest_sizes[alg_count].digest_size);
> +			alg_count++;
>  		}
>  	}
> +
> +	spec_event->number_of_algorithms = alg_count;
> +	if (spec_event->number_of_algorithms > MAX_HASH_COUNT ||
> +	    spec_event->number_of_algorithms < 1)
> +		goto out;
> +
>  	/*
>  	 * the size of the spec event and placement of vendor_info_size
>  	 * depends on supported algoriths
> @@ -1172,9 +1178,9 @@ static efi_status_t create_specid_event(struct udevice *dev, void *buffer,
>  		offsetof(struct tcg_efi_spec_id_event, digest_sizes) +
>  		spec_event->number_of_algorithms * sizeof(spec_event->digest_sizes[0]);
>  	/* no vendor info for us */
> -	memset(buffer + spec_event_size, 0,
> -	       sizeof(spec_event->vendor_info_size));
> -	spec_event_size += sizeof(spec_event->vendor_info_size);
> +	memset(buffer + spec_event_size, 0, 1);
> +	/* add a byte for vendor_info_size in the spec event */
> +	spec_event_size += 1;
>  	*event_size = spec_event_size;
>  
>  	return EFI_SUCCESS;
> -- 
> 2.25.1
> 

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>