From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takahiro Akashi <takahiro.akashi@linaro.org>
Date: Tue, 19 Mar 2019 09:19:08 +0900
Subject: [U-Boot] [PATCH 1/1] efi_loader: correct parameter size in
 efi_allocate_pool
In-Reply-To: <20190318193223.1725-1-xypron.glpk@gmx.de>
References: <20190318193223.1725-1-xypron.glpk@gmx.de>
Message-ID: <20190319001906.GD9937@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Mon, Mar 18, 2019 at 08:32:23PM +0100, Heinrich Schuchardt wrote:
> efi_allocate_pages() expects a (uint64_t *) pointer to pass the address of
> the assigned memory. If we pass the address of a pointer here, an illegal
> memory access occurs on 32bit systems.
> 
> Fixes: 282a06cbcae8 ("efi_loader: Expose U-Boot addresses in memory map
> for sandbox")
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_memory.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index ebd2b36c03..55622d2fb4 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -440,6 +440,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages)
>  efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
>  {
>  	efi_status_t r;
> +	u64 addr;
>  	struct efi_pool_allocation *alloc;
>  	u64 num_pages = efi_size_in_pages(size +
>  					  sizeof(struct efi_pool_allocation));
> @@ -453,9 +454,9 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer)
>  	}
>  
>  	r = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES, pool_type, num_pages,
> -			       (uint64_t *)&alloc);

I wonder why efi_allocate_pages() doesn't expect (void **) for the fourth
argument.
If this is because the type of the argument is a pointer to "physical address,"

> -
> +			       &addr);
>  	if (r == EFI_SUCCESS) {
> +		alloc = (struct efi_pool_allocation *)(uintptr_t)addr;

we should use map_sysmem() here.

Thanks,
-Takahiro Akashi

>  		alloc->num_pages = num_pages;
>  		*buffer = alloc->data;
>  	}
> -- 
> 2.20.1
>