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

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

[Borislav Petkov]

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@suse.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@suse.de>
---
 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

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

[Ard Biesheuvel]

On Wed, 30 Jun 2021 at 10:44, Borislav Petkov <bp@alien8.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@suse.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@suse.de>

Acked-by: Ard Biesheuvel <ardb@kernel.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

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

[Borislav Petkov]

On Fri, Jul 16, 2021 at 07:10:17PM +0200, Ard Biesheuvel wrote:
> Acked-by: Ard Biesheuvel <ardb@kernel.org>

Thanks for checking me here.

> Would you like me to queue this as a fix?

Well, I wanna say it is unnecessary to send it as an urgent fix because
we have the SEV fix in place now and I guess we'll leave it in in case
something else is in boot services and is needed for SEV guests to boot.

So I guess just a normal, unexpedited fix please, so that it sees more
testing before it goes up.

Thx Ard!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Ard Biesheuvel]

On Sat, 17 Jul 2021 at 06:46, Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Jul 16, 2021 at 07:10:17PM +0200, Ard Biesheuvel wrote:
> > Acked-by: Ard Biesheuvel <ardb@kernel.org>
>
> Thanks for checking me here.
>
> > Would you like me to queue this as a fix?
>
> Well, I wanna say it is unnecessary to send it as an urgent fix because
> we have the SEV fix in place now and I guess we'll leave it in in case
> something else is in boot services and is needed for SEV guests to boot.
>
> So I guess just a normal, unexpedited fix please, so that it sees more
> testing before it goes up.
>

I have accumulated a handful of EFI fixes which are not urgent, but
fixes nonetheless. I will add it to the set.