[PATCH] efi_loader: efi_allocate_pages: check parameter pages

[Peng Fan]

On 2/20/2023 4:08 AM, Heinrich Schuchardt wrote:
> On 2/16/23 11:53, Peng Fan (OSS) wrote:
>> From: Peng Fan <peng.fan at nxp.com>
>>
>> On i.MX8MM-EVK, when doing UEFI Capsule On Disk, we met such issue,
>> It will create Boot option for capsule on disk:
>> Boot0000: 
>> VenHw(E61D73B9-A384-4ACC-AEAB-82E828F3628B)/eMMC(0x2)/eMMC(0x1)/HD(1,GPT,F5CC8412-CD9F-4C9E-A782-0E945461E89E,0x800,0x32000)
>> But when capsule update finished, the updated image booting will trigger:
>> Loading Boot0000 'UEFI Capsule On Disk' failed
>> EFI boot manager: Cannot load any image
>> Found EFI removable media binary efi/boot/bootaa64.efi
>> "Synchronous Abort" handler, esr 0x96000004
>> elr: 000000004029f40c lr : 00000000402802f0 (reloc)
>> elr: 00000000bcd8b40c lr : 00000000bcd6c2f0
>> x0 : 02029ee86154940e x1 : 00000000bcd95458
>> x2 : 0000000000000010 x3 : 00000000bad31ad0
>> x4 : 0000000000000000 x5 : 02029ee86154940e
>> x6 : 0000000007f00000 x7 : 0000000000000007
>> x8 : 0000000000000009 x9 : 0000000000000008
>> x10: 0000000000000035 x11: 0000000000000010
>> x12: 0000000000000022 x13: 0000000000000001
>> x14: 00000000bacdedf0 x15: 0000000000000021
>> x16: 00000000bcd304d0 x17: 00000000000041a0
>> x18: 00000000bacebdb0 x19: 00000000b9c9f040
>> x20: 00000000bccecb28 x21: 00000000bcd95458
>> x22: 0000000000000001 x23: 00000000bad1f010
>> x24: 00000000bcdced70 x25: 0000000000001000
>> x26: 00000000b9c9e000 x27: 0000000040000000
>> x28: 0000000000000001 x29: 00000000bacdd030
>>
>> If is the pages is 0, the efi_find_free_memory will return the next used
>> memory, check the parameter pages, and return EFI_INVALID_PARAMETER if it
>> is 0.
>>
>> Reviewed-by: Ye Li <ye.li at nxp.com>
>> Reported-by: Vincent Stehlé <vincent.stehle at arm.com>
>> Signed-off-by: Peng Fan <peng.fan at nxp.com>
>> ---
>>   lib/efi_loader/efi_memory.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>> index b7bee98f79c..acca542033d 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -495,6 +495,9 @@ efi_status_t efi_allocate_pages(enum 
>> efi_allocate_type type,
>>       if (!memory)
>>           return EFI_INVALID_PARAMETER;
>>
>> +    if (!pages)
>> +        return EFI_INVALID_PARAMETER;
>> +
> 
> Looking at the UEFI specification this looks wrong. The EFI
> specification does not forbid calling AllocatePages() with pages == 0.
> So we should return EFI_SUCCESS.
> 
> EDK II returns EFI_NOT_FOUND for pages == 0. But this has no basis in
> the specification.
> 
> Which function is the caller invoking AllocatePages() with pages = 0.
> Where is the patch to fix it?


The call trace as below:
do_bootefi->do_efibootmgr->efi_bootmgr_load->try_load_entry
->efi_load_image->efi_load_image_from_path->efi_load_image_from_file
->efi_allocate_pages->efi_find_free_memory

There is an entry Boot0000:
VenHw(E61D73B9-A384-4ACC-AEAB-82E828F3628B)/eMMC(0x2)/eMMC(0x1)/HD(1,GPT,F5CC8412-CD9F-4C9E-A782-0E945461E89E,0x800,0x32000)

But the capsule1.bin has been removed during capsule update,
but the entry is still there.

The efi_file_size(f, &bs); will return bs as 0.

The issue is if len is 0, the efi_find_free_memory will return
memory that are already used by others.

Regards,
Peng.
> 
> Best regards
> 
> Heinrich
> 
>>       switch (type) {
>>       case EFI_ALLOCATE_ANY_PAGES:
>>           /* Any page */
>