arch/arm64/kernel/efi.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
KASAN reports invalid accesses during arch_stack_walk() for EFI runtime
services due to vmalloc tagging[1]. The EFI runtime stack must be allocated
with KASAN tags reset to avoid false positives.
This patch uses arch_alloc_vmap_stack() instead of __vmalloc_node() for
EFI stack allocation, which internally calls kasan_reset_tag()
The changes ensure EFI runtime stacks are properly sanitized for KASAN
while maintaining functional consistency.
Link: https://lore.kernel.org/all/aFVVEgD0236LdrL6@gmail.com/ [1]
Suggested-by: Andrey Konovalov <andreyknvl@gmail.com>
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
arch/arm64/kernel/efi.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 3857fd7ee8d46..d2af881a48290 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -15,6 +15,7 @@
#include <asm/efi.h>
#include <asm/stacktrace.h>
+#include <asm/vmap_stack.h>
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;
+
+ p = arch_alloc_vmap_stack(THREAD_SIZE, NUMA_NO_NODE);
+ if (!p) {
pr_warn("Failed to allocate EFI runtime stack\n");
clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
return -ENOMEM;
---
base-commit: a3e9ee4ad433efad9c172d5fcf63ff39b61c902f
change-id: 20250623-arm_kasan-3b1d120ec20f
Best regards,
--
Breno Leitao <leitao@debian.org>On Tue, Jun 24, 2025 at 05:55:53AM -0700, Breno Leitao wrote:
> KASAN reports invalid accesses during arch_stack_walk() for EFI runtime
> services due to vmalloc tagging[1]. The EFI runtime stack must be allocated
> with KASAN tags reset to avoid false positives.
>
> This patch uses arch_alloc_vmap_stack() instead of __vmalloc_node() for
> EFI stack allocation, which internally calls kasan_reset_tag()
>
> The changes ensure EFI runtime stacks are properly sanitized for KASAN
> while maintaining functional consistency.
>
> Link: https://lore.kernel.org/all/aFVVEgD0236LdrL6@gmail.com/ [1]
> Suggested-by: Andrey Konovalov <andreyknvl@gmail.com>
> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/arm64/kernel/efi.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 3857fd7ee8d46..d2af881a48290 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -15,6 +15,7 @@
>
> #include <asm/efi.h>
> #include <asm/stacktrace.h>
> +#include <asm/vmap_stack.h>
>
> 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.
> +
> + p = arch_alloc_vmap_stack(THREAD_SIZE, NUMA_NO_NODE);
> + if (!p) {
> pr_warn("Failed to allocate EFI runtime stack\n");
> clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> return -ENOMEM;
>
With that:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
(but let's see if Ard has a different opinion on the approach)
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:
> > KASAN reports invalid accesses during arch_stack_walk() for EFI runtime
> > services due to vmalloc tagging[1]. The EFI runtime stack must be allocated
> > with KASAN tags reset to avoid false positives.
> >
> > This patch uses arch_alloc_vmap_stack() instead of __vmalloc_node() for
> > EFI stack allocation, which internally calls kasan_reset_tag()
> >
> > The changes ensure EFI runtime stacks are properly sanitized for KASAN
> > while maintaining functional consistency.
> >
> > Link: https://lore.kernel.org/all/aFVVEgD0236LdrL6@gmail.com/ [1]
> > Suggested-by: Andrey Konovalov <andreyknvl@gmail.com>
> > Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> > arch/arm64/kernel/efi.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 3857fd7ee8d46..d2af881a48290 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -15,6 +15,7 @@
> >
> > #include <asm/efi.h>
> > #include <asm/stacktrace.h>
> > +#include <asm/vmap_stack.h>
> >
> > 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?
> > +
> > + p = arch_alloc_vmap_stack(THREAD_SIZE, NUMA_NO_NODE);
> > + if (!p) {
> > pr_warn("Failed to allocate EFI runtime stack\n");
> > clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> > return -ENOMEM;
> >
>
> With that:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
>
> (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.
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
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@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
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.
Any reason not to do that?
Will
On Fri, Jul 04, 2025 at 02:33:35PM +0100, Will Deacon wrote: > 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. > > Any reason not to do that? Not that I am aware of. I'm also in favour of unconditionally selecting VMAP_STACK. Mark.
On Fri, Jul 04, 2025 at 02:40:17PM +0100, Mark Rutland wrote: > On Fri, Jul 04, 2025 at 02:33:35PM +0100, Will Deacon wrote: > > 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. > > > > Any reason not to do that? > > Not that I am aware of. > > I'm also in favour of unconditionally selecting VMAP_STACK. So am I. -- Catalin
On Sun, Jul 06, 2025 at 07:45:04PM -0500, Catalin Marinas wrote: > On Fri, Jul 04, 2025 at 02:40:17PM +0100, Mark Rutland wrote: > > On Fri, Jul 04, 2025 at 02:33:35PM +0100, Will Deacon wrote: > > > 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. > > > > > > Any reason not to do that? > > > > Not that I am aware of. > > > > I'm also in favour of unconditionally selecting VMAP_STACK. > > So am I. Thanks. I've played a bit with it, and did some mechanical work, and send a v1. https://lore.kernel.org/all/20250707-arm64_vmap-v1-0-8de98ca0f91c@debian.org/
On Fri, 4 Jul 2025 at 15:33, Will Deacon <will@kernel.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@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
>
> 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.
© 2016 - 2025 Red Hat, Inc.