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) <[email protected]>
---
Issue is fixed with this patch.
Tested-by: Venkat Rao Bagalkote <[email protected]>
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
