[PATCH] powerpc/970: fix nap return address corruption on async interrupt exit

[PATCH] powerpc/970: fix nap return address corruption on async interrupt exit

[Mukesh Kumar Chaurasiya (IBM)]

On PowerMac G5 (PPC970, CONFIG_PPC_970_NAP) the system panics shortly
after boot with symptoms including instruction fetch faults, kernel data
access faults, and stack corruption, predominantly on SMP and always
somewhere inside softirq processing.

The PPC970 idle path works by setting _TLF_NAPPING in the current
thread's local flags before entering the MSR_POW nap loop.  When any
async interrupt wakes the CPU, nap_adjust_return() is expected to detect
_TLF_NAPPING, clear it, and rewrite regs->NIP to power4_idle_nap_return
so that the interrupt returns cleanly to the caller of power4_idle_nap()
rather than back into the nap spin loop.

DEFINE_INTERRUPT_HANDLER_ASYNC generates the following sequence:

    irq_enter_rcu();
    ____func(regs);           /* timer_interrupt / do_IRQ body */
    irq_exit_rcu();           /* softirqs run here, irqs re-enabled */
    arch_interrupt_async_exit_prepare(regs); /* nap_adjust_return was here */
    irqentry_exit(regs, state);

irq_exit_rcu() calls invoke_softirq() -> do_softirq_own_stack(), which
runs softirqs with hardware interrupts re-enabled.  A nested async
interrupt can therefore arrive while _TLF_NAPPING is still set.  That
nested interrupt reaches nap_adjust_return() in its own
arch_interrupt_async_exit_prepare() call, finds _TLF_NAPPING set, and
redirects *its own* regs->NIP to power4_idle_nap_return.  Returning via
that blr with an unrelated LR on the softirq stack jumps to a garbage
address, causing the observed crashes.

The comment that previously lived in arch_interrupt_async_exit_prepare()
even described this exact hazard ("must come before irq_exit()"), but
nap_adjust_return() was placed after irq_exit_rcu() in the macro, so
the protection was never effective.

Fix this by calling nap_adjust_return() inside DEFINE_INTERRUPT_HANDLER_ASYNC
immediately before irq_exit_rcu(), ensuring _TLF_NAPPING is cleared and
regs->NIP is adjusted before any code that can re-enable interrupts or
invoke softirqs runs.  Move the explanatory comment into
nap_adjust_return() itself and remove it from arch_interrupt_async_exit_prepare().

Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
Closes: https://lore.kernel.org/all/87wlvazrdy.fsf@igel.home/
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
---
 arch/powerpc/include/asm/entry-common.h | 15 +++++++--------
 arch/powerpc/include/asm/interrupt.h    |  1 +
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
index fc636c42e89a..c5adb5006361 100644
--- a/arch/powerpc/include/asm/entry-common.h
+++ b/arch/powerpc/include/asm/entry-common.h
@@ -66,6 +66,13 @@ static inline void srr_regs_clobbered(void)
 static inline void nap_adjust_return(struct pt_regs *regs)
 {
 #ifdef CONFIG_PPC_970_NAP
+	/*
+	 * Adjust the nap return address before irq_exit_rcu(). irq_exit_rcu()
+	 * may invoke softirqs with interrupts re-enabled, allowing a nested
+	 * async interrupt to arrive. If _TLF_NAPPING is still set at that
+	 * point, the nested interrupt would erroneously redirect its own
+	 * return address to power4_idle_nap_return, corrupting the stack.
+	 */
 	if (unlikely(test_thread_local_flags(_TLF_NAPPING))) {
 		/* Can avoid a test-and-clear because NMIs do not call this */
 		clear_thread_local_flags(_TLF_NAPPING);
@@ -286,14 +293,6 @@ static inline void arch_interrupt_async_enter_prepare(struct pt_regs *regs)
 
 static inline void arch_interrupt_async_exit_prepare(struct pt_regs *regs)
 {
-	/*
-	 * Adjust at exit so the main handler sees the true NIA. This must
-	 * come before irq_exit() because irq_exit can enable interrupts, and
-	 * if another interrupt is taken before nap_adjust_return has run
-	 * here, then that interrupt would return directly to idle nap return.
-	 */
-	nap_adjust_return(regs);
-
 	arch_interrupt_exit_prepare(regs);
 }
 
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index fb42a664ae54..1b45a49e9bed 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -246,6 +246,7 @@ interrupt_handler void func(struct pt_regs *regs)			\
 	instrumentation_begin();					\
 	irq_enter_rcu();						\
 	____##func (regs);						\
+	nap_adjust_return(regs);					\
 	irq_exit_rcu();							\
 	instrumentation_end();						\
 	arch_interrupt_async_exit_prepare(regs);			\
-- 
2.55.0

Re: [PATCH] powerpc/970: fix nap return address corruption on async interrupt exit

[Shrikanth Hegde]

Hi Mukesh.

On 7/7/26 10:54 PM, Mukesh Kumar Chaurasiya (IBM) wrote:
> On PowerMac G5 (PPC970, CONFIG_PPC_970_NAP) the system panics shortly
> after boot with symptoms including instruction fetch faults, kernel data
> access faults, and stack corruption, predominantly on SMP and always
> somewhere inside softirq processing.
> 
> The PPC970 idle path works by setting _TLF_NAPPING in the current
> thread's local flags before entering the MSR_POW nap loop.  When any
> async interrupt wakes the CPU, nap_adjust_return() is expected to detect
> _TLF_NAPPING, clear it, and rewrite regs->NIP to power4_idle_nap_return
> so that the interrupt returns cleanly to the caller of power4_idle_nap()
> rather than back into the nap spin loop.
> 
> DEFINE_INTERRUPT_HANDLER_ASYNC generates the following sequence:
> 
>      irq_enter_rcu();
>      ____func(regs);           /* timer_interrupt / do_IRQ body */
>      irq_exit_rcu();           /* softirqs run here, irqs re-enabled */
>      arch_interrupt_async_exit_prepare(regs); /* nap_adjust_return was here */
>      irqentry_exit(regs, state);
> 
> irq_exit_rcu() calls invoke_softirq() -> do_softirq_own_stack(), which
> runs softirqs with hardware interrupts re-enabled.  A nested async
> interrupt can therefore arrive while _TLF_NAPPING is still set.  That
> nested interrupt reaches nap_adjust_return() in its own
> arch_interrupt_async_exit_prepare() call, finds _TLF_NAPPING set, and
> redirects *its own* regs->NIP to power4_idle_nap_return.  Returning via
> that blr with an unrelated LR on the softirq stack jumps to a garbage
> address, causing the observed crashes.
> 
> The comment that previously lived in arch_interrupt_async_exit_prepare()
> even described this exact hazard ("must come before irq_exit()"), but
> nap_adjust_return() was placed after irq_exit_rcu() in the macro, so
> the protection was never effective.
> 
> Fix this by calling nap_adjust_return() inside DEFINE_INTERRUPT_HANDLER_ASYNC
> immediately before irq_exit_rcu(), ensuring _TLF_NAPPING is cleared and
> regs->NIP is adjusted before any code that can re-enable interrupts or
> invoke softirqs runs.  Move the explanatory comment into
> nap_adjust_return() itself and remove it from arch_interrupt_async_exit_prepare().
> 
> Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> Closes: https://lore.kernel.org/all/87wlvazrdy.fsf@igel.home/
> Reported-by: Andreas Schwab <schwab@linux-m68k.org>
> Signed-off-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
> ---
>   arch/powerpc/include/asm/entry-common.h | 15 +++++++--------
>   arch/powerpc/include/asm/interrupt.h    |  1 +
>   2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> index fc636c42e89a..c5adb5006361 100644
> --- a/arch/powerpc/include/asm/entry-common.h
> +++ b/arch/powerpc/include/asm/entry-common.h
> @@ -66,6 +66,13 @@ static inline void srr_regs_clobbered(void)
>   static inline void nap_adjust_return(struct pt_regs *regs)
>   {
>   #ifdef CONFIG_PPC_970_NAP
> +	/*
> +	 * Adjust the nap return address before irq_exit_rcu(). irq_exit_rcu()
> +	 * may invoke softirqs with interrupts re-enabled, allowing a nested
> +	 * async interrupt to arrive. If _TLF_NAPPING is still set at that
> +	 * point, the nested interrupt would erroneously redirect its own
> +	 * return address to power4_idle_nap_return, corrupting the stack.
> +	 */
>   	if (unlikely(test_thread_local_flags(_TLF_NAPPING))) {
>   		/* Can avoid a test-and-clear because NMIs do not call this */
>   		clear_thread_local_flags(_TLF_NAPPING);
> @@ -286,14 +293,6 @@ static inline void arch_interrupt_async_enter_prepare(struct pt_regs *regs)
>   
>   static inline void arch_interrupt_async_exit_prepare(struct pt_regs *regs)
>   {
> -	/*
> -	 * Adjust at exit so the main handler sees the true NIA. This must
> -	 * come before irq_exit() because irq_exit can enable interrupts, and
> -	 * if another interrupt is taken before nap_adjust_return has run
> -	 * here, then that interrupt would return directly to idle nap return.
> -	 */
> -	nap_adjust_return(regs);
> -
>   	arch_interrupt_exit_prepare(regs);
>   }

This makes arch_interrupt_async_exit_prepare same as arch_interrupt_exit_prepare.
Maybe remove arch_interrupt_async_exit_prepare?

>   
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index fb42a664ae54..1b45a49e9bed 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -246,6 +246,7 @@ interrupt_handler void func(struct pt_regs *regs)			\
>   	instrumentation_begin();					\
>   	irq_enter_rcu();						\
>   	____##func (regs);						\
> +	nap_adjust_return(regs);					\
>   	irq_exit_rcu();							\
>   	instrumentation_end();						\
>   	arch_interrupt_async_exit_prepare(regs);			\

Re: [PATCH] powerpc/970: fix nap return address corruption on async interrupt exit

[Mukesh Kumar Chaurasiya]

On Wed, Jul 08, 2026 at 09:57:44AM +0530, Shrikanth Hegde wrote:
> Hi Mukesh.
> 
> On 7/7/26 10:54 PM, Mukesh Kumar Chaurasiya (IBM) wrote:
<... snip ...>

> >   		/* Can avoid a test-and-clear because NMIs do not call this */
> >   		clear_thread_local_flags(_TLF_NAPPING);
> > @@ -286,14 +293,6 @@ static inline void arch_interrupt_async_enter_prepare(struct pt_regs *regs)
> >   static inline void arch_interrupt_async_exit_prepare(struct pt_regs *regs)
> >   {
> > -	/*
> > -	 * Adjust at exit so the main handler sees the true NIA. This must
> > -	 * come before irq_exit() because irq_exit can enable interrupts, and
> > -	 * if another interrupt is taken before nap_adjust_return has run
> > -	 * here, then that interrupt would return directly to idle nap return.
> > -	 */
> > -	nap_adjust_return(regs);
> > -
> >   	arch_interrupt_exit_prepare(regs);
> >   }
> 
> This makes arch_interrupt_async_exit_prepare same as arch_interrupt_exit_prepare.
> Maybe remove arch_interrupt_async_exit_prepare?
> 
Hey,

`arch_interrupt_async_exit_prepare` and `arch_interrupt_exit_prepare`,
both are marked static inline. So the generated assembly will have no
impact. I kept it this way coz it was consistent with how we implemented
other macros like `DEFINE_INTERRUPT_HANDLER`.

Regards,
Mukesh

> > diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> > index fb42a664ae54..1b45a49e9bed 100644
> > --- a/arch/powerpc/include/asm/interrupt.h
> > +++ b/arch/powerpc/include/asm/interrupt.h
> > @@ -246,6 +246,7 @@ interrupt_handler void func(struct pt_regs *regs)			\
> >   	instrumentation_begin();					\
> >   	irq_enter_rcu();						\
> >   	____##func (regs);						\
> > +	nap_adjust_return(regs);					\
> >   	irq_exit_rcu();							\
> >   	instrumentation_end();						\
> >   	arch_interrupt_async_exit_prepare(regs);			\
>