Re: [PATCH] efi: Free malloc regions on exit

[Andrei Borzenkov]

20.05.2016 07:34, Michael Chang пишет:
> On Fri, May 20, 2016 at 06:56:21AM +0300, Andrei Borzenkov wrote:
>> 19.05.2016 16:37, Alexander Graf пишет:
>>> When we exit grub, we don't free all the memory that we allocated earlier
>>> for our heap region. This can cause problems with setups where you try
>>> to descend the boot order using "exit" entries, such as PXE -> HD boot
>>> scenarios.
>>>
>>> Signed-off-by: Alexander Graf <address@hidden>
>>> ---
>>>  grub-core/kern/efi/init.c |  1 +
>>>  grub-core/kern/efi/mm.c   | 24 ++++++++++++++++++++++++
>>>  include/grub/efi/efi.h    |  1 +
>>>  3 files changed, 26 insertions(+)
>>>
>>> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
>>> index e9c85de..b848014 100644
>>> --- a/grub-core/kern/efi/init.c
>>> +++ b/grub-core/kern/efi/init.c
>>> @@ -77,4 +77,5 @@ grub_efi_fini (void)
>>>  {
>>>    grub_efidisk_fini ();
>>>    grub_console_fini ();
>>> +  grub_efi_memory_fini ();
>>>  }
>>
>> Note that grub_efi_fini() is called not only during exit, but also by
>> grub_loader_boot (grub_machine_fini); and - at least, theoretically -
>> grub_loader_boot_func can fail and we return back to GRUB. Which leaves
>> us with heap pointing to already freed area. We probably cannot do
>> anything useful at this point anyway, but this may lead to corruption of
>> memory allocated by other EFI drivers.
> 
> I think grub_machine_fini is called without GRUB_LOADER_FLAG_NORETURN flag set
> in above-mentioned case so that it should be fine. 
> 

Well, there are calls both with and without GRUB_LOADER_FLAG_NORETURN.
It is true that for EFI platform all *existing* calls are without, but
nothing really forces it. I am a bit uneasy about it. Ideally I'd prefer
clean code path that frees memory immediately before existing GRUB.

> Thanks,
> Michael
> 
>>
>> May be it should be called explicitly only in exit path.
>>
>> Also it is not called during chainload at all, which should have the
>> same problem (i.e. conceptually it does not matter whether we exit grub
>> and select next binary from EFI menu or simply try to chainload it from
>> grub).
>>

No comments?