Re: [PATCH v7 1/3] efi/x86: Fix EFI memory map corruption with kexec

[Dave Young]

On Wed, 5 Jun 2024 at 10:09, Kalra, Ashish <ashish.kalra@xxxxxxx> wrote:
>
> Hello Dave,
>
> On 6/4/2024 8:58 PM, Dave Young wrote:
> > On Wed, 5 Jun 2024 at 09:52, Dave Young <dyoung@xxxxxxxxxx> wrote:
> >>>>         ...
> >>>>         if (efi.memmap.flags & (EFI_MEMMAP_MEMBLOCK | EFI_MEMMAP_SLAB)) {
> >>>>                 __efi_memmap_free(efi.memmap.phys_map,
> >>>>                                 efi.memmap.desc_size * efi.memmap.nr_map, efi.memmap.flags);
> >>>>         }
> >>> From your debugging the memmap should not be freed.  This piece of
> >>> code was added in below commit,  added Dan Williams in cc list:
> >>> commit f0ef6523475f18ccd213e22ee593dfd131a2c5ea
> >>> Author: Dan Williams <dan.j.williams@xxxxxxxxx>
> >>> Date:   Mon Jan 13 18:22:44 2020 +0100
> >>>
> >>>     efi: Fix efi_memmap_alloc() leaks
> >>>
> >>>     With efi_fake_memmap() and efi_arch_mem_reserve() the efi table may be
> >>>     updated and replaced multiple times. When that happens a previous
> >>>     dynamically allocated efi memory map can be garbage collected. Use the
> >>>     new EFI_MEMMAP_{SLAB,MEMBLOCK} flags to detect when a dynamically
> >>>     allocated memory map is being replaced.
> >>>
> >> Dan, probably those regions should be freed only for "fake" memmap?
> > Ashish, can you comment out the __efi_memmap_free see if it works for
> > you just confirm about the behavior.
>
> Yes, i have already tried and tested that, if i avoid __efi_memmap_free(), then i don't see this memory map corruption.

Ok, thanks!  I think the right way is creating two patches,  one to
remove the __efi_memmap_free, another is  skip efi_arch_mem_reserve
when the EFI_MEMORY_RUNTIME bit was set already.  But the first one
should be the fix for the root cause.

efi fake mem is only for debugging purposes,  the "memleak" mentioned
in commit 0f96a99dab36 should be solved in another way if needed (are
they really leaked? or just not useful anymore)

Anyway this is my opinion, please wait for x86 and efi reviewer's inputs.

>
> Thanks, Ashish
>