arm64: panic if IRQ shadow call stack allocation fails

[PATCH] arm64: panic if IRQ shadow call stack allocation fails

Posted by Osama Abdelkader 1 week, 4 days ago

scs_alloc() can return NULL when vmalloc fails. init_irq_scs() previously
stored that NULL in per-cpu irq_shadow_call_stack_ptr, which IRQ entry
would then use under CONFIG_SHADOW_CALL_STACK. Match other SCS setup paths
(e.g. SDEI) by failing explicitly instead of continuing with a NULL
pointer.

Mark init_irq_scs() __init since it is only called from init_IRQ().

Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 arch/arm64/kernel/irq.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index 15dedb385b9e..b32ed7ef8e00 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -14,6 +14,7 @@
 #include <linux/init.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
+#include <linux/kernel.h>
 #include <linux/kprobes.h>
 #include <linux/memory.h>
 #include <linux/scs.h>
@@ -32,23 +33,26 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
 
 DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
 
-
 DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
 
 #ifdef CONFIG_SHADOW_CALL_STACK
 DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
 #endif
 
-static void init_irq_scs(void)
+static void __init init_irq_scs(void)
 {
 	int cpu;
+	void *s;
 
 	if (!scs_is_enabled())
 		return;
 
-	for_each_possible_cpu(cpu)
-		per_cpu(irq_shadow_call_stack_ptr, cpu) =
-			scs_alloc(early_cpu_to_node(cpu));
+	for_each_possible_cpu(cpu) {
+		s = scs_alloc(early_cpu_to_node(cpu));
+		if (!s)
+			panic("irq: Failed to allocate shadow call stack\n");
+		per_cpu(irq_shadow_call_stack_ptr, cpu) = s;
+	}
 }
 
 static void __init init_irq_stacks(void)
-- 
2.43.0

Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails

Posted by Will Deacon 1 week, 3 days ago

On Tue, Mar 24, 2026 at 05:15:41PM +0100, Osama Abdelkader wrote:
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 15dedb385b9e..b32ed7ef8e00 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -14,6 +14,7 @@
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/irqchip.h>
> +#include <linux/kernel.h>
>  #include <linux/kprobes.h>
>  #include <linux/memory.h>
>  #include <linux/scs.h>
> @@ -32,23 +33,26 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
>  
>  DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
>  
> -
>  DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
>  
>  #ifdef CONFIG_SHADOW_CALL_STACK
>  DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
>  #endif
>  
> -static void init_irq_scs(void)
> +static void __init init_irq_scs(void)
>  {
>  	int cpu;
> +	void *s;
>  
>  	if (!scs_is_enabled())
>  		return;
>  
> -	for_each_possible_cpu(cpu)
> -		per_cpu(irq_shadow_call_stack_ptr, cpu) =
> -			scs_alloc(early_cpu_to_node(cpu));
> +	for_each_possible_cpu(cpu) {
> +		s = scs_alloc(early_cpu_to_node(cpu));
> +		if (!s)
> +			panic("irq: Failed to allocate shadow call stack\n");
> +		per_cpu(irq_shadow_call_stack_ptr, cpu) = s;
> +	}

I don't especially see the point in these panic() messages given that
presumably all sorts of other things will go wrong if we fail simple
allocations this early during boot.

If you really want to check this, then we should at least do the same
for the IRQ stack itself, otherwise it's all a bit academic. So maybe
have init_irq_scs() and init_irq_stacks() return -ENOMEM so that
init_IRQ() can panic?

Will

Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails

Posted by Osama Abdelkader 1 week, 2 days ago

On Wed, Mar 25, 2026 at 04:35:35PM +0000, Will Deacon wrote:
> On Tue, Mar 24, 2026 at 05:15:41PM +0100, Osama Abdelkader wrote:
> > diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> > index 15dedb385b9e..b32ed7ef8e00 100644
> > --- a/arch/arm64/kernel/irq.c
> > +++ b/arch/arm64/kernel/irq.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/init.h>
> >  #include <linux/irq.h>
> >  #include <linux/irqchip.h>
> > +#include <linux/kernel.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/memory.h>
> >  #include <linux/scs.h>
> > @@ -32,23 +33,26 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
> >  
> >  DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
> >  
> > -
> >  DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> >  
> >  #ifdef CONFIG_SHADOW_CALL_STACK
> >  DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> >  #endif
> >  
> > -static void init_irq_scs(void)
> > +static void __init init_irq_scs(void)
> >  {
> >  	int cpu;
> > +	void *s;
> >  
> >  	if (!scs_is_enabled())
> >  		return;
> >  
> > -	for_each_possible_cpu(cpu)
> > -		per_cpu(irq_shadow_call_stack_ptr, cpu) =
> > -			scs_alloc(early_cpu_to_node(cpu));
> > +	for_each_possible_cpu(cpu) {
> > +		s = scs_alloc(early_cpu_to_node(cpu));
> > +		if (!s)
> > +			panic("irq: Failed to allocate shadow call stack\n");
> > +		per_cpu(irq_shadow_call_stack_ptr, cpu) = s;
> > +	}
> 
> I don't especially see the point in these panic() messages given that
> presumably all sorts of other things will go wrong if we fail simple
> allocations this early during boot.
> 
> If you really want to check this, then we should at least do the same
> for the IRQ stack itself, otherwise it's all a bit academic. So maybe
> have init_irq_scs() and init_irq_stacks() return -ENOMEM so that
> init_IRQ() can panic?
> 
> Will

Thanks for the review, I just did that in v2:
[PATCH v2] arm64: panic from init_IRQ if IRQ handler stacks cannot be
 allocated

Best regards,
Osama

Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails

Posted by Breno Leitao 1 week, 4 days ago

On Tue, Mar 24, 2026 at 05:15:41PM +0100, Osama Abdelkader wrote:
> scs_alloc() can return NULL when vmalloc fails. init_irq_scs() previously
> stored that NULL in per-cpu irq_shadow_call_stack_ptr, which IRQ entry
> would then use under CONFIG_SHADOW_CALL_STACK. Match other SCS setup paths
> (e.g. SDEI) by failing explicitly instead of continuing with a NULL
> pointer.

Right,  _init_sdei_scs() doesn't not assign the per cpu pointer with
NULL, but, at the same time it doesn't panic. SDEI propagates -ENOMEM
back up the call chain and even frees already allocated stacks via
free_sdei_scs(). Should it panic as well?

> Mark init_irq_scs() __init since it is only called from init_IRQ().
> 
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
>  arch/arm64/kernel/irq.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index 15dedb385b9e..b32ed7ef8e00 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -14,6 +14,7 @@
>  #include <linux/init.h>
>  #include <linux/irq.h>
>  #include <linux/irqchip.h>
> +#include <linux/kernel.h>

Why do you need kernel.h in here? I initially thought it was
for panic(), but, later I found panic() is already in use in this file.

Isn't kernel.h being included transitively?
>  #include <linux/kprobes.h>
>  #include <linux/memory.h>
>  #include <linux/scs.h>
> @@ -32,23 +33,26 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
>  
>  DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
>  
> -
>  DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
>  
>  #ifdef CONFIG_SHADOW_CALL_STACK
>  DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
>  #endif
>  
> -static void init_irq_scs(void)
> +static void __init init_irq_scs(void)
>  {
>  	int cpu;
> +	void *s;
>  
>  	if (!scs_is_enabled())
>  		return;
>  
> -	for_each_possible_cpu(cpu)
> -		per_cpu(irq_shadow_call_stack_ptr, cpu) =
> -			scs_alloc(early_cpu_to_node(cpu));
> +	for_each_possible_cpu(cpu) {
> +		s = scs_alloc(early_cpu_to_node(cpu));
> +		if (!s)
> +			panic("irq: Failed to allocate shadow call stack\n");
> +		per_cpu(irq_shadow_call_stack_ptr, cpu) = s;
> +	}
>  }

Reading RISC-V code, it seems it has the same problem. Is it worth fixing also?

 static void init_irq_scs(void)
  {
          int cpu;

          if (!scs_is_enabled())
                  return;

          for_each_possible_cpu(cpu)
                  per_cpu(irq_shadow_call_stack_ptr, cpu) =
                          scs_alloc(cpu_to_node(cpu));
  }

Other than these nits, feel free to add:

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails

Posted by Osama Abdelkader 1 week, 2 days ago

On Wed, Mar 25, 2026 at 01:54:32AM -0700, Breno Leitao wrote:
> On Tue, Mar 24, 2026 at 05:15:41PM +0100, Osama Abdelkader wrote:
> > scs_alloc() can return NULL when vmalloc fails. init_irq_scs() previously
> > stored that NULL in per-cpu irq_shadow_call_stack_ptr, which IRQ entry
> > would then use under CONFIG_SHADOW_CALL_STACK. Match other SCS setup paths
> > (e.g. SDEI) by failing explicitly instead of continuing with a NULL
> > pointer.
> 
> Right,  _init_sdei_scs() doesn't not assign the per cpu pointer with
> NULL, but, at the same time it doesn't panic. SDEI propagates -ENOMEM
> back up the call chain and even frees already allocated stacks via
> free_sdei_scs(). Should it panic as well?
>

Thanks, I changed it to return -ENOMEM in v2 to address will's review.
 
> > Mark init_irq_scs() __init since it is only called from init_IRQ().
> > 
> > Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> > ---
> >  arch/arm64/kernel/irq.c | 14 +++++++++-----
> >  1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> > index 15dedb385b9e..b32ed7ef8e00 100644
> > --- a/arch/arm64/kernel/irq.c
> > +++ b/arch/arm64/kernel/irq.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/init.h>
> >  #include <linux/irq.h>
> >  #include <linux/irqchip.h>
> > +#include <linux/kernel.h>
> 
> Why do you need kernel.h in here? I initially thought it was
> for panic(), but, later I found panic() is already in use in this file.
> 
> Isn't kernel.h being included transitively?

Right, I removed it in v2, thanks.

> >  #include <linux/kprobes.h>
> >  #include <linux/memory.h>
> >  #include <linux/scs.h>
> > @@ -32,23 +33,26 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
> >  
> >  DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
> >  
> > -
> >  DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> >  
> >  #ifdef CONFIG_SHADOW_CALL_STACK
> >  DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> >  #endif
> >  
> > -static void init_irq_scs(void)
> > +static void __init init_irq_scs(void)
> >  {
> >  	int cpu;
> > +	void *s;
> >  
> >  	if (!scs_is_enabled())
> >  		return;
> >  
> > -	for_each_possible_cpu(cpu)
> > -		per_cpu(irq_shadow_call_stack_ptr, cpu) =
> > -			scs_alloc(early_cpu_to_node(cpu));
> > +	for_each_possible_cpu(cpu) {
> > +		s = scs_alloc(early_cpu_to_node(cpu));
> > +		if (!s)
> > +			panic("irq: Failed to allocate shadow call stack\n");
> > +		per_cpu(irq_shadow_call_stack_ptr, cpu) = s;
> > +	}
> >  }
> 
> Reading RISC-V code, it seems it has the same problem. Is it worth fixing also?
> 
>  static void init_irq_scs(void)
>   {
>           int cpu;
> 
>           if (!scs_is_enabled())
>                   return;
> 
>           for_each_possible_cpu(cpu)
>                   per_cpu(irq_shadow_call_stack_ptr, cpu) =
>                           scs_alloc(cpu_to_node(cpu));
>   }

Yes, thanks for the check.

> 
> Other than these nits, feel free to add:
> 
> Reviewed-by: Breno Leitao <leitao@debian.org>

Thank you. I sent v2:
[PATCH v2] arm64: panic from init_IRQ if IRQ handler stacks cannot be
 allocated

To cover init_irq_stacks as well.

Best regards,
Osama