]> www.infradead.org Git - users/jedix/linux-maple.git/commitdiff
kaiser: paranoid_entry pass cr3 need to paranoid_exit
authorHugh Dickins <hughd@google.com>
Wed, 27 Sep 2017 01:43:07 +0000 (18:43 -0700)
committerKirtikar Kashyap <kirtikar.kashyap@oracle.com>
Fri, 12 Jan 2018 18:20:08 +0000 (10:20 -0800)
Neel Natu points out that paranoid_entry() was wrong to assume that
an entry that did not need swapgs would not need SWITCH_KERNEL_CR3:
paranoid_entry (used for debug breakpoint, int3, double fault or MCE;
though I think it's only the MCE case that is cause for concern here)
can break in at an awkward time, between cr3 switch and swapgs, but
its handling always needs kernel gs and kernel cr3.

Easy to fix in itself, but paranoid_entry() also needs to convey to
paranoid_exit() (and my reading of macro idtentry says paranoid_entry
and paranoid_exit are always paired) how to restore the prior state.
The swapgs state is already conveyed by %ebx (0 or 1), so extend that
also to convey when SWITCH_USER_CR3 will be needed (2 or 3).

(Yes, I'd much prefer that 0 meant no swapgs, whereas it's the other
way round: and a convention shared with error_entry() and error_exit(),
which I don't want to touch.  Perhaps I should have inverted the bit
for switch cr3 too, but did not.)

paranoid_exit() would be straightforward, except for TRACE_IRQS: it
did TRACE_IRQS_IRETQ when doing swapgs, but TRACE_IRQS_IRETQ_DEBUG
when not: which is it supposed to use when SWITCH_USER_CR3 is split
apart from that?  As best as I can determine, commit 5963e317b1e9
("ftrace/x86: Do not change stacks in DEBUG when calling lockdep")
missed the swapgs case, and should have used TRACE_IRQS_IRETQ_DEBUG
there too (the discrepancy has nothing to do with the liberal use
of _NO_STACK and _UNSAFE_STACK hereabouts: TRACE_IRQS_OFF_DEBUG has
just been used in all cases); discrepancy lovingly preserved across
several paranoid_exit() cleanups, but I'm now removing it.

Neel further indicates that to use SWITCH_USER_CR3_NO_STACK there in
paranoid_exit() is now not only unnecessary but unsafe: might corrupt
syscall entry's unsafe_stack_register_backup of %rax.  Just use
SWITCH_USER_CR3: and delete SWITCH_USER_CR3_NO_STACK altogether,
before we make the mistake of using it again.

hughd adds: this commit fixes an issue in the Kaiser-without-PCIDs
part of the series, and ought to be moved earlier, if you decided
to make a release of Kaiser-without-PCIDs.

Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit fc8334e6b3e5d28afd4eec8a74493933f73b2784)
Orabug: 27333760
CVE: CVE-2017-5754
Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
Conflicts:
arch/x86/entry/entry_64.S (not in this tree)
arch/x86/kernel/entry_64.S (patched instead of that)

Signed-off-by: Kirtikar Kashyap <kirtikar.kashyap@oracle.com>
arch/x86/include/asm/kaiser.h
arch/x86/kernel/entry_64.S

index 48d8d70dd8c72d8834d222af831d7c754ccdbebf..3dc5f4c39b3e86d6d1bc6fc6ff9815db8f8221a7 100644 (file)
@@ -63,20 +63,12 @@ _SWITCH_TO_KERNEL_CR3 %rax
 movq PER_CPU_VAR(unsafe_stack_register_backup), %rax
 .endm
 
-.macro SWITCH_USER_CR3_NO_STACK
-movq %rax, PER_CPU_VAR(unsafe_stack_register_backup)
-_SWITCH_TO_USER_CR3 %rax %al
-movq PER_CPU_VAR(unsafe_stack_register_backup), %rax
-.endm
-
 #else /* CONFIG_KAISER */
 
 .macro SWITCH_KERNEL_CR3 reg
 .endm
 .macro SWITCH_USER_CR3 reg regb
 .endm
-.macro SWITCH_USER_CR3_NO_STACK
-.endm
 .macro SWITCH_KERNEL_CR3_NO_STACK
 .endm
 
index caf851d7491df6c5fb6d5ddac93a2933b33e68db..2ccab5479ad4ca81fd49250df746d3185d7b84a7 100644 (file)
@@ -1469,7 +1469,11 @@ idtentry machine_check has_error_code=0 paranoid=1 do_sym=*machine_check_vector(
 /*
  * Save all registers in pt_regs, and switch gs if needed.
  * Use slow, but surefire "are we in kernel?" check.
- * Return: ebx=0: need swapgs on exit, ebx=1: otherwise
+ *
+ * Return: ebx=0: needs swapgs but not SWITCH_USER_CR3 in paranoid_exit
+ *         ebx=1: needs neither swapgs nor SWITCH_USER_CR3 in paranoid_exit
+ *         ebx=2: needs both swapgs and SWITCH_USER_CR3 in paranoid_exit
+ *         ebx=3: needs SWITCH_USER_CR3 but not swapgs in paranoid_exit
  */
 ENTRY(paranoid_entry)
        XCPT_FRAME 1 15*8
@@ -1490,9 +1494,25 @@ ENTRY(paranoid_entry)
        testl %edx,%edx
        js 1f   /* negative -> in kernel */
        SWAPGS
-       SWITCH_KERNEL_CR3
        xorl %ebx,%ebx
 1:     ENABLE_IBRS_SAVE_AND_CLOBBER save_reg=%r13d
+#ifdef CONFIG_KAISER
+       /*
+        * We might have come in between a swapgs and a SWITCH_KERNEL_CR3
+        * on entry, or between a SWITCH_USER_CR3 and a swapgs on exit.
+        * Do a conditional SWITCH_KERNEL_CR3: this could safely be done
+        * unconditionally, but we need to find out whether the reverse
+        * should be done on return (conveyed to paranoid_exit in %ebx).
+        */
+       movq    %cr3, %rax
+       testl   $KAISER_SHADOW_PGD_OFFSET, %eax
+       jz      2f
+       orl     $2, %ebx
+       andq    $(~(X86_CR3_PCID_ASID_MASK | KAISER_SHADOW_PGD_OFFSET)), %rax
+       orq     x86_cr3_pcid_noflush, %rax
+       movq    %rax, %cr3
+2:
+#endif
        ret
        CFI_ENDPROC
 END(paranoid_entry)
@@ -1506,22 +1526,27 @@ END(paranoid_entry)
  * in syscall entry), so checking for preemption here would
  * be complicated.  Fortunately, we there's no good reason
  * to try to handle preemption here.
+ * On entry: ebx=0: needs swapgs but not SWITCH_USER_CR3
+ *           ebx=1: needs neither swapgs nor SWITCH_USER_CR3
+ *           ebx=2: needs both swapgs and SWITCH_USER_CR3
+ *           ebx=3: needs SWITCH_USER_CR3 but not swapgs
  */
-/* On entry, ebx is "no swapgs" flag (1: don't need swapgs, 0: need it) */
 ENTRY(paranoid_exit)
        DEFAULT_FRAME
        DISABLE_INTERRUPTS(CLBR_NONE)
        TRACE_IRQS_OFF_DEBUG
-       testl %ebx,%ebx                         /* swapgs needed? */
+       TRACE_IRQS_IRETQ_DEBUG
+#ifdef CONFIG_KAISER
+       testl   $2, %ebx                        /* SWITCH_USER_CR3 needed? */
+       jz      paranoid_exit_no_switch
+       SWITCH_USER_CR3
+paranoid_exit_no_switch:
+#endif
+       testl   $1, %ebx                        /* swapgs needed? */
        jnz paranoid_exit_no_swapgs
-       TRACE_IRQS_IRETQ
        RESTORE_IBRS_CLOBBER save_reg=%r13d
-       SWITCH_USER_CR3_NO_STACK
        SWAPGS_UNSAFE_STACK
-       jmp paranoid_exit_restore
 paranoid_exit_no_swapgs:
-       TRACE_IRQS_IRETQ_DEBUG
-paranoid_exit_restore:
        RESTORE_EXTRA_REGS
        RESTORE_C_REGS
        REMOVE_PT_GPREGS_FROM_STACK 8