From: Osama Abdelkader <osama.abdelkader@gmail.com>
To: Breno Leitao <leitao@debian.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Ard Biesheuvel <ardb@kernel.org>,
Ryo Takakura <ryotkkr98@gmail.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: panic if IRQ shadow call stack allocation fails
Date: Fri, 27 Mar 2026 00:02:14 +0100 [thread overview]
Message-ID: <acW69iCN8l2clhc-@osama> (raw)
In-Reply-To: <acOhjkLqf2gsM-Sf@gmail.com>
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
next prev parent reply other threads:[~2026-03-26 23:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2026-03-25 16:35 ` Will Deacon
2026-03-26 23:03 ` Osama Abdelkader
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acW69iCN8l2clhc-@osama \
--to=osama.abdelkader@gmail.com \
--cc=ardb@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=leitao@debian.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=ryotkkr98@gmail.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.