Re: [PATCH v6 2/5] efi/boot.c: add file.need_to_free

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.09.2020 12:45, Roger Pau Monné wrote:
> On Mon, Sep 21, 2020 at 07:51:10AM -0400, Trammell Hudson wrote:
>> --- a/xen/common/efi/boot.c
>> +++ b/xen/common/efi/boot.c
>> @@ -102,6 +102,7 @@ union string {
>>  
>>  struct file {
>>      UINTN size;
>> +    bool need_to_free;
>>      union {
>>          EFI_PHYSICAL_ADDRESS addr;
>>          char *str;
>> @@ -280,13 +281,13 @@ void __init noreturn blexit(const CHAR16 *str)
>>      if ( !efi_bs )
>>          efi_arch_halt();
>>  
>> -    if ( cfg.addr )
>> +    if ( cfg.need_to_free )
> 
> If you drop the addr check here...
> 
>>          efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>> -    if ( kernel.addr )
>> +    if ( kernel.need_to_free )
>>          efi_bs->FreePages(kernel.addr, PFN_UP(kernel.size));
>> -    if ( ramdisk.addr )
>> +    if ( ramdisk.need_to_free )
>>          efi_bs->FreePages(ramdisk.addr, PFN_UP(ramdisk.size));
>> -    if ( xsm.addr )
>> +    if ( xsm.need_to_free )
>>          efi_bs->FreePages(xsm.addr, PFN_UP(xsm.size));
>>  
>>      efi_arch_blexit();
>> @@ -581,6 +582,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, 
>> CHAR16 *name,
>>      }
>>      else
>>      {
>> +        file->need_to_free = true;
> 
> ... I think you need to clear need_to_free if AllocatePages fails?

But this has been moved to the success path, or am I overlooking
anything?

Jan