u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [PATCH] efi_loader: Fix spec ID event creation
@ 2021-09-14  6:44 Ruchika Gupta
  2021-09-14  6:50 ` Ilias Apalodimas
  0 siblings, 1 reply; 2+ messages in thread
From: Ruchika Gupta @ 2021-09-14  6:44 UTC (permalink / raw)
  To: u-boot
  Cc: Ruchika Gupta, Masahisa Kojima, Ilias Apalodimas, Heinrich Schuchardt

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@linaro.org>
CC: Masahisa Kojima <masahisa.kojima@linaro.org>
CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
CC: Heinrich Schuchardt <xypron.glpk@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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] efi_loader: Fix spec ID event creation
  2021-09-14  6:44 [PATCH] efi_loader: Fix spec ID event creation Ruchika Gupta
@ 2021-09-14  6:50 ` Ilias Apalodimas
  0 siblings, 0 replies; 2+ messages in thread
From: Ilias Apalodimas @ 2021-09-14  6:50 UTC (permalink / raw)
  To: Ruchika Gupta; +Cc: u-boot, Masahisa Kojima, Heinrich Schuchardt

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@linaro.org>
> CC: Masahisa Kojima <masahisa.kojima@linaro.org>
> CC: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> CC: Heinrich Schuchardt <xypron.glpk@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@linaro.org>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-09-14 10:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14  6:44 [PATCH] efi_loader: Fix spec ID event creation Ruchika Gupta
2021-09-14  6:50 ` Ilias Apalodimas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).