diff mbox series

powerpc: Do not restore KUAP in arch_exit_to_user_mode_prepare()

Message ID 52fee44fd23acf8e1c024ace668728e626a783a8.1788101609.git.ritesh.list@gmail.com (mailing list archive)
State Under Review
Headers show
Series powerpc: Do not restore KUAP in arch_exit_to_user_mode_prepare() | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_ppctests success Successfully ran 10 jobs.
snowpatch_ozlabs/github-powerpc_selftests success Successfully ran 10 jobs.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.
snowpatch_ozlabs/github-powerpc_kernel_qemu success Successfully ran 22 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 5 jobs.

Commit Message

Ritesh Harjani (IBM) Aug. 30, 2026, 2:54 p.m. UTC 
KUAP means kernel cannot touch user memory unless it explicitly is
enabled. In the kernel it should stay AMR_KUAP_BLOCKED. While returning
to userspace just before RFI, kernel should restore the user AMR value
back.

Looks like GENERIC_ENTRY might be treating arch_exit_to_user_mode_prepare()
as the last architecture step before returning to userspace.
commit bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
therefore called kuap_user_restore() from that hook. But on PowerPC that
is too early. After irqentry_exit() / syscall_exit_to_user_mode() we
still run platform specific exit routines.

e.g. code snippets showing both exception handling and system call
handling as the callers of function arch_exit_to_user_mode_prepare()
which does kuap_user_restore(). The below path shows that calling
kuap_user_restore() is too early when called from
arch_exit_to_user_mode_prepare().

Exception handling in exceptions-64s.S
=======================================

bl	CFUNC(do_page_fault)
	  ..DEFINE_INTERRUPT_HANDLER_ASYNC(do_page_fault)
	      arch_interrupt_async_enter_prepare(regs);
	      state = irqentry_enter(regs);
	      instrumentation_begin();
	      irq_enter_rcu();
	      handler(regs);
	      nap_adjust_return(regs);
	      irq_exit_rcu();
	      instrumentation_end();
	      arch_interrupt_async_exit_prepare(regs);
	      irqentry_exit(regs, state); 		   <<< too early
	        irqentry_exit_to_user_mode()
		  __exit_to_user_mode_prepare(regs, EXIT_TO_USER_MODE_WORK_IRQ);
		    arch_exit_to_user_mode_prepare(regs, ti_work);  <<< too early
b	interrupt_return_srr
		.. bl	CFUNC(interrupt_exit_user_prepare) <<< already calls kuap_user_restore

prep_irq_for_enabled_exit() retry can run kernel code with IRQs on. So
only when that routine is fully finished is when the user KUAP should be
fully restored which interrupt_exit_user_prepare() already takes care of
before returning.

Similarly for system call handling in interrupt_64.S
======================================================

	bl	CFUNC(system_call_exception)

.Lsyscall_exit:
	addi	r4,r1,STACK_INT_FRAME_REGS
	li	r5,0 /* !scv */
	bl	CFUNC(syscall_exit_prepare)
		  .. kuap_assert_locked();
		     syscall_exit_to_user_mode(regs); <<< too early
		       syscall_exit_to_user_mode_prepare(regs);  <<< too early
 		     kuap_user_restore(regs);         <<< already calls

syscall_exit_prepare(), which can enable IRQs, replay a pending
interrupt, and only then rfi. Those functions already restore KUAP
immediately before rfi.

Note that if we restore the user AMR too early like in the current code
as shown from the code snippets above, then we get the following warning
when CONFIG_PPC_KUAP_DEBUG is enabled:
  WARNING: arch/powerpc/include/asm/book3s/64/kup.h:293 at interrupt_exit_user_prepare+0x1a0/0x1c0
  Hardware name: IBM pSeries (emulated by qemu) POWER10 (architected)
  TRAP: 0700
  LR: c00000000000d8d4 CTR: c0000000021fe500
  MSR: <SF,EE,ME,IR,DR,RI,LE>  CR: 44000804  XER: 20040000
  interrupt_exit_user_prepare+0x1a0/0x1c0
  interrupt_return_srr_user+0x8/0x12c

Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
Fixes: 02565a782c1ee ("powerpc: Introduce syscall exit arch functions")
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 arch/powerpc/include/asm/entry-common.h | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

--
2.39.5

Comments

Mukesh Kumar Chaurasiya Aug. 31, 2026, 4:11 a.m. UTC | #1 
On Sun, Aug 30, 2026 at 08:24:30PM +0530, Ritesh Harjani (IBM) wrote:
> KUAP means kernel cannot touch user memory unless it explicitly is
> enabled. In the kernel it should stay AMR_KUAP_BLOCKED. While returning
> to userspace just before RFI, kernel should restore the user AMR value
> back.
> 
> Looks like GENERIC_ENTRY might be treating arch_exit_to_user_mode_prepare()
> as the last architecture step before returning to userspace.
> commit bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> therefore called kuap_user_restore() from that hook. But on PowerPC that
> is too early. After irqentry_exit() / syscall_exit_to_user_mode() we
> still run platform specific exit routines.
> 
> e.g. code snippets showing both exception handling and system call
> handling as the callers of function arch_exit_to_user_mode_prepare()
> which does kuap_user_restore(). The below path shows that calling
> kuap_user_restore() is too early when called from
> arch_exit_to_user_mode_prepare().
> 
> Exception handling in exceptions-64s.S
> =======================================
> 
> bl	CFUNC(do_page_fault)
> 	  ..DEFINE_INTERRUPT_HANDLER_ASYNC(do_page_fault)
> 	      arch_interrupt_async_enter_prepare(regs);
> 	      state = irqentry_enter(regs);
> 	      instrumentation_begin();
> 	      irq_enter_rcu();
> 	      handler(regs);
> 	      nap_adjust_return(regs);
> 	      irq_exit_rcu();
> 	      instrumentation_end();
> 	      arch_interrupt_async_exit_prepare(regs);
> 	      irqentry_exit(regs, state); 		   <<< too early
> 	        irqentry_exit_to_user_mode()
> 		  __exit_to_user_mode_prepare(regs, EXIT_TO_USER_MODE_WORK_IRQ);
> 		    arch_exit_to_user_mode_prepare(regs, ti_work);  <<< too early
> b	interrupt_return_srr
> 		.. bl	CFUNC(interrupt_exit_user_prepare) <<< already calls kuap_user_restore
> 
> prep_irq_for_enabled_exit() retry can run kernel code with IRQs on. So
> only when that routine is fully finished is when the user KUAP should be
> fully restored which interrupt_exit_user_prepare() already takes care of
> before returning.
> 
> Similarly for system call handling in interrupt_64.S
> ======================================================
> 
> 	bl	CFUNC(system_call_exception)
> 
> .Lsyscall_exit:
> 	addi	r4,r1,STACK_INT_FRAME_REGS
> 	li	r5,0 /* !scv */
> 	bl	CFUNC(syscall_exit_prepare)
> 		  .. kuap_assert_locked();
> 		     syscall_exit_to_user_mode(regs); <<< too early
> 		       syscall_exit_to_user_mode_prepare(regs);  <<< too early
>  		     kuap_user_restore(regs);         <<< already calls
> 
> syscall_exit_prepare(), which can enable IRQs, replay a pending
> interrupt, and only then rfi. Those functions already restore KUAP
> immediately before rfi.
> 
> Note that if we restore the user AMR too early like in the current code
> as shown from the code snippets above, then we get the following warning
> when CONFIG_PPC_KUAP_DEBUG is enabled:
>   WARNING: arch/powerpc/include/asm/book3s/64/kup.h:293 at interrupt_exit_user_prepare+0x1a0/0x1c0
>   Hardware name: IBM pSeries (emulated by qemu) POWER10 (architected)
>   TRAP: 0700
>   LR: c00000000000d8d4 CTR: c0000000021fe500
>   MSR: <SF,EE,ME,IR,DR,RI,LE>  CR: 44000804  XER: 20040000
>   interrupt_exit_user_prepare+0x1a0/0x1c0
>   interrupt_return_srr_user+0x8/0x12c
> 
> Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> Fixes: 02565a782c1ee ("powerpc: Introduce syscall exit arch functions")
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---
>  arch/powerpc/include/asm/entry-common.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> index c5adb5006361..94083516df57 100644
> --- a/arch/powerpc/include/asm/entry-common.h
> +++ b/arch/powerpc/include/asm/entry-common.h
> @@ -515,8 +515,14 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>  	local_paca->tm_scratch = regs->msr;
>  #endif
> -	/* Restore user access locks last */
> -	kuap_user_restore(regs);
> +	/*
> +	 * Do not restore KUAP here. Generic entry might treat this as the last
> +	 * arch step before userspace but PowerPC still has kernel work after
> +	 * irqentry_exit()/syscall_exit_to_user_mode() i.e. in
> +	 * interrupt_exit_user_prepare() / syscall_exit_prepare() may enable
> +	 * IRQs and retry. Those functions restore KUAP immediately before rfi,
> +	 * which is where it should belong.
> +	 */
>  }
> 
>  #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
> --
> 2.39.5
> 
> 
Yeah, KUAP access should be restored last.

Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
Venkat Sept. 1, 2026, 4:27 a.m. UTC | #2 
On 30/08/26 8:24 pm, Ritesh Harjani (IBM) wrote:
> KUAP means kernel cannot touch user memory unless it explicitly is
> enabled. In the kernel it should stay AMR_KUAP_BLOCKED. While returning
> to userspace just before RFI, kernel should restore the user AMR value
> back.
>
> Looks like GENERIC_ENTRY might be treating arch_exit_to_user_mode_prepare()
> as the last architecture step before returning to userspace.
> commit bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> therefore called kuap_user_restore() from that hook. But on PowerPC that
> is too early. After irqentry_exit() / syscall_exit_to_user_mode() we
> still run platform specific exit routines.
>
> e.g. code snippets showing both exception handling and system call
> handling as the callers of function arch_exit_to_user_mode_prepare()
> which does kuap_user_restore(). The below path shows that calling
> kuap_user_restore() is too early when called from
> arch_exit_to_user_mode_prepare().
>
> Exception handling in exceptions-64s.S
> =======================================
>
> bl	CFUNC(do_page_fault)
> 	  ..DEFINE_INTERRUPT_HANDLER_ASYNC(do_page_fault)
> 	      arch_interrupt_async_enter_prepare(regs);
> 	      state = irqentry_enter(regs);
> 	      instrumentation_begin();
> 	      irq_enter_rcu();
> 	      handler(regs);
> 	      nap_adjust_return(regs);
> 	      irq_exit_rcu();
> 	      instrumentation_end();
> 	      arch_interrupt_async_exit_prepare(regs);
> 	      irqentry_exit(regs, state); 		   <<< too early
> 	        irqentry_exit_to_user_mode()
> 		  __exit_to_user_mode_prepare(regs, EXIT_TO_USER_MODE_WORK_IRQ);
> 		    arch_exit_to_user_mode_prepare(regs, ti_work);  <<< too early
> b	interrupt_return_srr
> 		.. bl	CFUNC(interrupt_exit_user_prepare) <<< already calls kuap_user_restore
>
> prep_irq_for_enabled_exit() retry can run kernel code with IRQs on. So
> only when that routine is fully finished is when the user KUAP should be
> fully restored which interrupt_exit_user_prepare() already takes care of
> before returning.
>
> Similarly for system call handling in interrupt_64.S
> ======================================================
>
> 	bl	CFUNC(system_call_exception)
>
> .Lsyscall_exit:
> 	addi	r4,r1,STACK_INT_FRAME_REGS
> 	li	r5,0 /* !scv */
> 	bl	CFUNC(syscall_exit_prepare)
> 		  .. kuap_assert_locked();
> 		     syscall_exit_to_user_mode(regs); <<< too early
> 		       syscall_exit_to_user_mode_prepare(regs);  <<< too early
>   		     kuap_user_restore(regs);         <<< already calls
>
> syscall_exit_prepare(), which can enable IRQs, replay a pending
> interrupt, and only then rfi. Those functions already restore KUAP
> immediately before rfi.
>
> Note that if we restore the user AMR too early like in the current code
> as shown from the code snippets above, then we get the following warning
> when CONFIG_PPC_KUAP_DEBUG is enabled:
>    WARNING: arch/powerpc/include/asm/book3s/64/kup.h:293 at interrupt_exit_user_prepare+0x1a0/0x1c0
>    Hardware name: IBM pSeries (emulated by qemu) POWER10 (architected)
>    TRAP: 0700
>    LR: c00000000000d8d4 CTR: c0000000021fe500
>    MSR: <SF,EE,ME,IR,DR,RI,LE>  CR: 44000804  XER: 20040000
>    interrupt_exit_user_prepare+0x1a0/0x1c0
>    interrupt_return_srr_user+0x8/0x12c
>
> Fixes: bee25f97ad24 ("powerpc: Enable GENERIC_ENTRY feature")
> Fixes: 02565a782c1ee ("powerpc: Introduce syscall exit arch functions")
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---

Issue is fixed with this patch.

Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>


Before this Patch:


[    3.861094] ------------[ cut here ]------------
[    3.861105] WARNING: ./arch/powerpc/include/asm/book3s/64/kup.h:293 
at interrupt_exit_user_prepare+0x240/0x260, CPU#23: modprobe/348
[    3.861124] Modules linked in:
[    3.861133] CPU: 23 UID: 0 PID: 348 Comm: modprobe Not tainted 7.2.0+ 
#14 PREEMPTLAZY
[    3.861145] Hardware name: IBM,8375-42A POWER9 (architected) 0x4e0202 
0xf000005 of:IBM,FW950.80 (VL950_131) hv:phyp pSeries
[    3.861154] NIP:  c00000000003a730 LR: c00000000003a53c CTR: 
000000000049f42c
[    3.861162] REGS: c0000000a7967b70 TRAP: 0700   Not tainted (7.2.0+)
[    3.861170] MSR:  8000000000029033 <SF,EE,ME,IR,DR,RI,LE> CR: 
44000008  XER: 20040000
[    3.861197] CFAR: c00000000003a56c IRQMASK: 1
                GPR00: c00000000003a53c c0000000a7967e10 
c000000002b7a400 c0000000a7967fb8
                GPR04: 800000004000d033 0000000000000001 
c0000000000a7460 ffffffffffffffbf
                GPR08: 013ffffe00040038 fcffffffffffffff 
3cffffffffffffff 0000000000000000
                GPR12: c0000000a7967688 c000000017fe3700 
0000000000000000 0000000000000000
                GPR16: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR20: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR24: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR28: 0000000000000000 0000000000000000 
0000000000000000 c0000000a7967e80
[    3.861313] NIP [c00000000003a730] 
interrupt_exit_user_prepare+0x240/0x260
[    3.861325] LR [c00000000003a53c] interrupt_exit_user_prepare+0x4c/0x260
[    3.861336] Call Trace:
[    3.861340] [c0000000a7967e10] [c00000000003a53c] 
interrupt_exit_user_prepare+0x4c/0x260 (unreliable)
[    3.861356] [c0000000a7967e50] [c00000000000d8d4] 
interrupt_return_srr_user+0x8/0x12c
[    3.861370] ---- interrupt: 400 at 0x7fff8cf341c0
[    3.861379] NIP:  00007fff8cf341c0 LR: 0000000000000000 CTR: 
0000000000000000
[    3.861388] REGS: c0000000a7967e80 TRAP: 0400   Not tainted (7.2.0+)
[    3.861395] MSR:  800000004000d033 <SF,EE,PR,ME,IR,DR,RI,LE>  CR: 
00000000  XER: 00000000
[    3.861424] CFAR: c00000000000d7bc IRQMASK: 0
                GPR00: 0000000000000000 00007fffc7b04020 
0000000000000000 0000000000000000
                GPR04: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR08: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR12: 00007fff8cf341c0 0000000000000000 
0000000000000000 0000000000000000
                GPR16: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR20: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR24: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
                GPR28: 0000000000000000 0000000000000000 
0000000000000000 0000000000000000
[    3.861536] NIP [00007fff8cf341c0] 0x7fff8cf341c0
[    3.861543] LR [0000000000000000] 0x0
[    3.861550] ---- interrupt: 400
[    3.861555] Code: 7fbd0aa6 387f0178 488befad 60000000 e93f0178 
7c3d4800 4082febc 73de00ff 4182fef8 3ba00000 4bfffeb4 60000000 
<0fe00000> 4bfffe3c 60000000 60000000

[    3.861599] ---[ end trace 0000000000000000 ]---


Regards,

Venkat.

>   arch/powerpc/include/asm/entry-common.h | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
> index c5adb5006361..94083516df57 100644
> --- a/arch/powerpc/include/asm/entry-common.h
> +++ b/arch/powerpc/include/asm/entry-common.h
> @@ -515,8 +515,14 @@ static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>   #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>   	local_paca->tm_scratch = regs->msr;
>   #endif
> -	/* Restore user access locks last */
> -	kuap_user_restore(regs);
> +	/*
> +	 * Do not restore KUAP here. Generic entry might treat this as the last
> +	 * arch step before userspace but PowerPC still has kernel work after
> +	 * irqentry_exit()/syscall_exit_to_user_mode() i.e. in
> +	 * interrupt_exit_user_prepare() / syscall_exit_prepare() may enable
> +	 * IRQs and retry. Those functions restore KUAP immediately before rfi,
> +	 * which is where it should belong.
> +	 */
>   }
>
>   #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
> --
> 2.39.5
>
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/entry-common.h b/arch/powerpc/include/asm/entry-common.h
index c5adb5006361..94083516df57 100644
--- a/arch/powerpc/include/asm/entry-common.h
+++ b/arch/powerpc/include/asm/entry-common.h
@@ -515,8 +515,14 @@  static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 	local_paca->tm_scratch = regs->msr;
 #endif
-	/* Restore user access locks last */
-	kuap_user_restore(regs);
+	/*
+	 * Do not restore KUAP here. Generic entry might treat this as the last
+	 * arch step before userspace but PowerPC still has kernel work after
+	 * irqentry_exit()/syscall_exit_to_user_mode() i.e. in
+	 * interrupt_exit_user_prepare() / syscall_exit_prepare() may enable
+	 * IRQs and retry. Those functions restore KUAP immediately before rfi,
+	 * which is where it should belong.
+	 */
 }

 #define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare