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.gu...@linaro.org>
> CC: Masahisa Kojima <masahisa.koj...@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> CC: Heinrich Schuchardt <xypron.g...@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.apalodi...@linaro.org>

Reply via email to