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

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: <CAMj1kXFUjJmJjR3b2S8pZeEheKojJGCYjRWRQDS0EbDYhGEUVw@mail.gmail.com>
Date: Fri, 4 Jul 2025 15:37:29 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Will Deacon <will@...nel.org>
Cc: Breno Leitao <leitao@...ian.org>, Catalin Marinas <catalin.marinas@....com>, 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

On Fri, 4 Jul 2025 at 15:33, Will Deacon <will@...nel.org> wrote:
>
> On Fri, Jul 04, 2025 at 01:36:40PM +0100, Breno Leitao wrote:
> > 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
>
> I would actually like to select VMAP_STACK unconditionally for arm64.
> Historically, we were held back waiting for all the various KASAN modes
> to support vmalloc properly, but I _think_ that's fixed now...
>
> The VMAP_STACK dependency is:
>
>         depends on !KASAN || KASAN_HW_TAGS || KASAN_VMALLOC
>
> and in arm64 we have:
>
>         select KASAN_VMALLOC if KASAN
>
> so it should be fine to select it afaict.
>

I agree - we have plenty of vmalloc space, and the memory footprint is
the same so we should just turn this on unconditionally.