* [PATCH] arm64: panic if IRQ shadow call stack allocation fails @ 2026-03-24 16:15 Osama Abdelkader 2026-03-25 8:54 ` Breno Leitao 2026-03-25 16:35 ` Will Deacon 0 siblings, 2 replies; 5+ messages in thread From: Osama Abdelkader @ 2026-03-24 16:15 UTC (permalink / raw) To: Catalin Marinas, Will Deacon, Mark Rutland, Osama Abdelkader, Ard Biesheuvel, Breno Leitao, Ryo Takakura, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails 2026-03-24 16:15 [PATCH] arm64: panic if IRQ shadow call stack allocation fails Osama Abdelkader @ 2026-03-25 8:54 ` Breno Leitao 2026-03-26 23:02 ` Osama Abdelkader 2026-03-25 16:35 ` Will Deacon 1 sibling, 1 reply; 5+ messages in thread From: Breno Leitao @ 2026-03-25 8:54 UTC (permalink / raw) To: Osama Abdelkader Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel, Ryo Takakura, linux-arm-kernel, linux-kernel 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> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails 2026-03-25 8:54 ` Breno Leitao @ 2026-03-26 23:02 ` Osama Abdelkader 0 siblings, 0 replies; 5+ messages in thread From: Osama Abdelkader @ 2026-03-26 23:02 UTC (permalink / raw) To: Breno Leitao Cc: Catalin Marinas, Will Deacon, Mark Rutland, Ard Biesheuvel, Ryo Takakura, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails 2026-03-24 16:15 [PATCH] arm64: panic if IRQ shadow call stack allocation fails Osama Abdelkader 2026-03-25 8:54 ` Breno Leitao @ 2026-03-25 16:35 ` Will Deacon 2026-03-26 23:03 ` Osama Abdelkader 1 sibling, 1 reply; 5+ messages in thread From: Will Deacon @ 2026-03-25 16:35 UTC (permalink / raw) To: Osama Abdelkader Cc: Catalin Marinas, Mark Rutland, Ard Biesheuvel, Breno Leitao, Ryo Takakura, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails 2026-03-25 16:35 ` Will Deacon @ 2026-03-26 23:03 ` Osama Abdelkader 0 siblings, 0 replies; 5+ messages in thread From: Osama Abdelkader @ 2026-03-26 23:03 UTC (permalink / raw) To: Will Deacon Cc: Catalin Marinas, Mark Rutland, Ard Biesheuvel, Breno Leitao, Ryo Takakura, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-26 23:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-24 16:15 [PATCH] arm64: panic if IRQ shadow call stack allocation fails Osama Abdelkader 2026-03-25 8:54 ` Breno Leitao 2026-03-26 23:02 ` Osama Abdelkader 2026-03-25 16:35 ` Will Deacon 2026-03-26 23:03 ` Osama Abdelkader
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox