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

[Simon Glass]

Hi Alex,

On 18 June 2018 at 09:08, Alexander Graf <agraf at suse.de> wrote:
> 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.

We actually have no choice but to support this. As mentioned elsewhere
addresses are the main currency in U-Boot and we should not look to
convert it to use pointers. They are much less enjoyable to work with.
The above patch is pretty simple.

Regards,
Simon