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: <aGfK2N6po39zyVIp@gmail.com>
Date: Fri, 4 Jul 2025 13:36:40 +0100
From: Breno Leitao <leitao@...ian.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, usamaarif642@...il.com,
	rmikey@...a.com, andreyknvl@...il.com, kasan-dev@...glegroups.com,
	linux-efi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH] arm64: efi: Fix KASAN false positive for EFI runtime
 stack

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@....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@...nel.org>
	Signed-off-by: Breno Leitao <leitao@...ian.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ