Re: [PATCH] powerpc: Do not restore KUAP in arch_exit_to_user_mode_prepare()

[Venkat Rao Bagalkote <venkat88@linux.ibm.com>]

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
>