[U-Boot] [PATCH v8 29/30] efi_loader: Pass address to fs_read()

[Alexander Graf]

On 06/18/2018 04:08 PM, Simon Glass wrote:
> From: Alexander Graf <agraf at suse.de>
>
> The fs_read() function wants to get an address rather than the
> pointer to a buffer.
>
> So let's convert the passed buffer from pointer back a the address
> to make efi_loader on sandbox happier.
>
> Signed-off-by: Alexander Graf <agraf at suse.de>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
>   lib/efi_loader/efi_file.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index e6a15bcb52..2107730ba5 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -9,6 +9,7 @@
>   #include <charset.h>
>   #include <efi_loader.h>
>   #include <malloc.h>
> +#include <mapmem.h>
>   #include <fs.h>
>   
>   /* GUID for file system information */
> @@ -232,8 +233,10 @@ static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
>   		void *buffer)
>   {
>   	loff_t actread;
> +	/* fs_read expects buffer address, not pointer */
> +	uintptr_t buffer_addr = (uintptr_t)map_to_sysmem(buffer);

As you've seen with your patch on the stack based map_to_sysmem() 
calculation, map_to_sysmem() really should only be used on pointers that 
we are sure come from RAM. With EFI binaries, that isn't true because of 
the stack, but it might be true due to other reasons too on real 
hardware, such as direct read/write to/from flash.

I think it's much safer to stay in pointer land once we passed the addr 
-> ptr boundary. Going from ptr -> addr is usually a recipe for disaster.


Alex