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]>
---
 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


Reply via email to