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 <ag...@suse.de>
>>> ---
>>> 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?
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
