Re: [tip:efi/core] efi/esrt: Only call efi_mem_reserve() for boot services memory

[Ard Biesheuvel]

Hello Ingo,

Thanks for pulling this directly. I will have another batch for
efi/core by the end of the week (and another urgent patch as well).

There is one remaining issue with mixed mode that I cc'ed you and
Kirill on, but since that seems to be a v4.17 regression, I will keep
that as a separate thing.

-- 
Ard.


On 17 July 2018 at 17:24, tip-bot for Ard Biesheuvel <tipbot@xxxxxxxxx> wrote:
> Commit-ID:  61f0d55569463a1af897117ff47d202b0ccb2e24
> Gitweb:     https://git.kernel.org/tip/61f0d55569463a1af897117ff47d202b0ccb2e24
> Author:     Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> AuthorDate: Mon, 16 Jul 2018 23:25:07 +0800
> Committer:  Ingo Molnar <mingo@xxxxxxxxxx>
> CommitDate: Tue, 17 Jul 2018 09:15:05 +0200
>
> efi/esrt: Only call efi_mem_reserve() for boot services memory
>
> The following commit:
>
>   7e1550b8f208 ("efi: Drop type and attribute checks in efi_mem_desc_lookup()")
>
> refactored the implementation of efi_mem_desc_lookup() so that the type
> check is moved to the callers, one of which is the x86 version of
> efi_arch_mem_reserve(), where we added a modified check that only takes
> EFI_BOOT_SERVICES_DATA regions into account.
>
> This is reasonable, since it is the only memory type that requires this,
> but doing so uncovered some unexpected behavior in the ESRT code, which
> permits the ESRT table to reside in other types of memory than what the
> UEFI spec mandates (i.e., EFI_BOOT_SERVICES_DATA), and unconditionally
> calls efi_mem_reserve() on the region in question. This may result in
> errors such as
>
>   esrt: Reserving ESRT space from 0x000000009c810318 to 0x000000009c810350.
>   efi: Failed to lookup EFI memory descriptor for 0x000000009c810318
>
> when the ESRT table is not in EFI_BOOT_SERVICES_DATA memory, but we try
> to reserve it nonetheless.
>
> So make the call to efi_mem_reserve() conditional on the memory type.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Peter Jones <pjones@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: linux-efi@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
>  drivers/firmware/efi/esrt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
> index 375a77c1c6e5..5d06bd247d07 100644
> --- a/drivers/firmware/efi/esrt.c
> +++ b/drivers/firmware/efi/esrt.c
> @@ -329,7 +329,8 @@ void __init efi_esrt_init(void)
>
>         end = esrt_data + size;
>         pr_info("Reserving ESRT space from %pa to %pa.\n", &esrt_data, &end);
> -       efi_mem_reserve(esrt_data, esrt_data_size);
> +       if (md.type == EFI_BOOT_SERVICES_DATA)
> +               efi_mem_reserve(esrt_data, esrt_data_size);
>
>         pr_debug("esrt-init: loaded.\n");
>  }