linux-kernel - Re: [RFC PATCH] efi/mokvar: Reserve the table only if it is in boot services data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMj1kXE--H53wu_X=GpgeJmWs7vjpnkUnG_fc+59GaNDF+sYEw@mail.gmail.com>
Date:   Fri, 16 Jul 2021 19:10:17 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Lenny Szubowicz <lszubowi@...hat.com>, Gary Lin <glin@...e.com>,
        Jörg Rödel <jroedel@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        linux-efi <linux-efi@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] efi/mokvar: Reserve the table only if it is in boot
 services data

On Wed, 30 Jun 2021 at 10:44, Borislav Petkov <bp@...en8.de> wrote:
>
> Hi guys,
>
> so below is what we've been staring at recently, please doublecheck me
> whether I'm even making sense here.
>
> Thx!
>
> ---
> From: Borislav Petkov <bp@...e.de>
>
> One of the SUSE QA tests triggered:
>
>   localhost kernel: efi: Failed to lookup EFI memory descriptor for 0x000000003dcf8000
>
> which comes from x86's version of efi_arch_mem_reserve() trying to
> reserve a memory region. Usually, that function expects
> EFI_BOOT_SERVICES_DATA memory descriptors but the above case is for the
> MOKvar table which is allocated in the EFI shim as runtime services.
>
> That lead to a fix changing the allocation of that table to boot services.
>
> However, that fix broke booting SEV guests with that shim leading to
> this kernel fix
>
>   8d651ee9c71b ("x86/ioremap: Map EFI-reserved memory as encrypted for SEV")
>
> which extended the ioremap hint to map reserved EFI boot services as
> decrypted too.
>
> However, all that wasn't needed, IMO, because that error message in
> efi_arch_mem_reserve() was innocuous in this case - if the MOKvar table
> is not in boot services, then it doesn't need to be reserved in the
> first place because it is, well, in runtime services which *should* be
> reserved anyway.
>
> So do that reservation for the MOKvar table only if it is allocated
> in boot services data. I couldn't find any requirement about where
> that table should be allocated in, unlike the ESRT which allocation is
> mandated to be done in boot services data by the UEFI spec.
>
> Signed-off-by: Borislav Petkov <bp@...e.de>

Acked-by: Ard Biesheuvel <ardb@...nel.org>

Would you like me to queue this as a fix?

> ---
>  drivers/firmware/efi/mokvar-table.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
> index d8bc01340686..38722d2009e2 100644
> --- a/drivers/firmware/efi/mokvar-table.c
> +++ b/drivers/firmware/efi/mokvar-table.c
> @@ -180,7 +180,10 @@ void __init efi_mokvar_table_init(void)
>                 pr_err("EFI MOKvar config table is not valid\n");
>                 return;
>         }
> -       efi_mem_reserve(efi.mokvar_table, map_size_needed);
> +
> +       if (md.type == EFI_BOOT_SERVICES_DATA)
> +               efi_mem_reserve(efi.mokvar_table, map_size_needed);
> +
>         efi_mokvar_table_size = map_size_needed;
>  }
>
> --
> 2.29.2
>
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette