Le 04/02/2021 à 04:27, Nicholas Piggin a écrit :
Excerpts from Christophe Leroy's message of February 4, 2021 2:25 am:
Le 25/02/2020 à 18:35, Nicholas Piggin a écrit :
Implement the bulk of interrupt return logic in C. The asm return code
must handle a few cases: restoring full GPRs, and emulating stack store.
+notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs,
unsigned long msr)
+{
+ unsigned long *ti_flagsp = ¤t_thread_info()->flags;
+ unsigned long flags;
+
+ if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
+ unrecoverable_exception(regs);
+ BUG_ON(regs->msr & MSR_PR);
+ BUG_ON(!FULL_REGS(regs));
+
+ local_irq_save(flags);
+
+ if (regs->softe == IRQS_ENABLED) {
+ /* Returning to a kernel context with local irqs enabled. */
+ WARN_ON_ONCE(!(regs->msr & MSR_EE));
+again:
+ if (IS_ENABLED(CONFIG_PREEMPT)) {
+ /* Return to preemptible kernel context */
+ if (unlikely(*ti_flagsp & _TIF_NEED_RESCHED)) {
+ if (preempt_count() == 0)
+ preempt_schedule_irq();
+ }
+ }
+
+ trace_hardirqs_on();
+ __hard_EE_RI_disable();
+ if (unlikely(lazy_irq_pending())) {
+ __hard_RI_enable();
+ irq_soft_mask_set(IRQS_ALL_DISABLED);
+ trace_hardirqs_off();
+ local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
+ /*
+ * Can't local_irq_enable in case we are in interrupt
+ * context. Must replay directly.
+ */
+ replay_soft_interrupts();
+ irq_soft_mask_set(flags);
+ /* Took an interrupt, may have more exit work to do. */
+ goto again;
+ }
+ local_paca->irq_happened = 0;
+ irq_soft_mask_set(IRQS_ENABLED);
+ } else {
+ /* Returning to a kernel context with local irqs disabled. */
+ trace_hardirqs_on();
+ __hard_EE_RI_disable();
+ if (regs->msr & MSR_EE)
+ local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
+ }
+
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ local_paca->tm_scratch = regs->msr;
+#endif
+
+ /*
+ * We don't need to restore AMR on the way back to userspace for KUAP.
+ * The value of AMR only matters while we're in the kernel.
+ */
+ kuap_restore_amr(regs);
Is that correct to restore KUAP state here ? Shouldn't we have it at lower
level in assembly ?
Isn't there a risk that someone manages to call interrupt_exit_kernel_prepare()
or the end of it in
a way or another, and get the previous KUAP state restored by this way ?
I'm not sure if there much more risk if it's here rather than the
instruction being in another place in the code.
There's a lot of user access around the kernel too if you want to find a
gadget to unlock KUAP then I suppose there is a pretty large attack
surface.
My understanding is that user access scope is strictly limited, for instance we enforce the
begin/end of user access to be in the same function, and we refrain from calling any other function
inside the user access window. x86 even have 'objtool' to enforce it at build time. So in theory
there is no way to get out of the function while user access is open.
Here with the interrupt exit function it is free beer. You have a place where you re-open user
access and return with a simple blr. So that's open bar. If someone manages to just call the
interrupt exit function, then user access remains open
Also, it looks a bit strange to have kuap_save_amr_and_lock() done at lowest
level in assembly, and
kuap_restore_amr() done in upper level. That looks unbalanced.
I'd like to bring the entry assembly into C.
I really think it's not a good idea. We'll get better control and readability by keeping it at the
lowest possible level in assembly.
x86 only save and restore SMAC state in assembly.
Christophe
