Re: [PATCH v2] efi_loader: Fix memory corruption on 32bit systems

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hi Dan, 

[...]

> @@ -313,7 +313,7 @@ static int cmp_pe_section(const void *arg1, const void *arg2)
>   *
>   * Return:	valid pointer to a image, return NULL if allocation fails.
>   */
> -void *efi_prepare_aligned_image(void *efi, u64 *efi_size)
> +void *efi_prepare_aligned_image(void *efi, size_t *efi_size)
>  {
>  	size_t new_efi_size;
>  	void *new_efi;
> @@ -600,7 +600,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
>  	if (!efi_secure_boot_enabled())
>  		return true;
>  
> -	new_efi = efi_prepare_aligned_image(efi, (u64 *)&efi_size);
> +	new_efi = efi_prepare_aligned_image(efi, &efi_size);
>  	if (!new_efi)
>  		return false;
>  
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 49f8a5e77cbf..d57afd0c498b 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -882,7 +882,7 @@ out:
>   *
>   * Return:	status code
>   */
> -static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size,
> +static efi_status_t tcg2_hash_pe_image(void *efi, size_t efi_size,
>  				       struct tpml_digest_values *digest_list)

Unfortunately the rabbit hole is a bit deeper with this one.
tcg2_hash_pe_image() is called in 
- tcg2_measure_pe_image(). This one is called in efi_load_pe() and the type
  is indeed a size_t there, so that's fine
- efi_tcg2_hash_log_extend_event(), this one is different...
The function is described by the EFI spec [0] which mandates a u64... I
think that was the reason efi_prepare_aligned_image() is using a u64 to
begin with.  This one uses the size only though not the pointer, but in a
32bit platform it would truncate s size > UINT_MAX.

[0] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf

Regards
/Ilias
>  {
>  	WIN_CERTIFICATE *wincerts = NULL;
> -- 
> 2.39.2
>