Re: [PATCH] arm64: efi: Fix KASAN false positive for EFI runtime stack

[Breno Leitao <leitao@debian.org>]

Hello Ard,

On Fri, Jul 04, 2025 at 10:26:37AM +0200, Ard Biesheuvel wrote:
> On Thu, 3 Jul 2025 at 18:35, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tue, Jun 24, 2025 at 05:55:53AM -0700, Breno Leitao wrote:
...
> > >  arch/arm64/kernel/efi.c | 9 ++++++---
...
> > >  static bool region_is_misaligned(const efi_memory_desc_t *md)
> > >  {
> > > @@ -214,9 +215,11 @@ static int __init arm64_efi_rt_init(void)
> > >       if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > >               return 0;
> > >
> > > -     p = __vmalloc_node(THREAD_SIZE, THREAD_ALIGN, GFP_KERNEL,
> > > -                        NUMA_NO_NODE, &&l);
> > > -l:   if (!p) {
> > > +     if (!IS_ENABLED(CONFIG_VMAP_STACK))
> > > +             return -ENOMEM;
> >
> > Mark Rutland pointed out in a private chat that this should probably
> > clear the EFI_RUNTIME_SERVICES flag as well.
> >
> 
> If VMAP_STACK is a hard requirement, should we make CONFIG_EFI depend
> on it for arm64?

What about if we make CONFIG_EFI select VMAP_STACK? I think it is more
straight forward from a configuration perspective.

I thought about the following. What do you think?

	arm64: EFI selects VMAP_STACK

	Modify the ARM64 Kconfig to make the CONFIG_EFI configuration option
	automatically select CONFIG_VMAP_STACK.

	The motivation is that arm64_efi_rt_init() will fail at runtime if
	CONFIG_VMAP_STACK is not set, so the patch ensures that enabling EFI
	will always enable VMAP_STACK as well, and avoid having EFI disabled in
	case the user didn't set VMAP_STACK.

	Suggested-by: Ard Biesheuvel <ardb@kernel.org>
	Signed-off-by: Breno Leitao <leitao@debian.org>

	diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
	index 55fc331af3371..cc2585143f511 100644
	--- a/arch/arm64/Kconfig
	+++ b/arch/arm64/Kconfig
	@@ -2437,6 +2437,7 @@ config EFI
		select EFI_RUNTIME_WRAPPERS
		select EFI_STUB
		select EFI_GENERIC_STUB
	+	select VMAP_STACK
		imply IMA_SECURE_AND_OR_TRUSTED_BOOT
		default y
		help

> > (but let's see if Ard has a different opinion on the approach)

> I think this is fine - the stack just needs to be disjoint from the
> ordinary kernel mode task stack so that buggy firmware is less likely
> to corrupt it, and so that we can recover from an unexpected
> synchronous exception more reliably.
> 
> In that sense, the old and the new code are equivalent, so no
> objections from me.

Thanks. I will send an update with the update that Catalin and Mark
suggested.

Thanks!
--breno