x86/entry: Vastly simplify SYSENTER TF (single-step) handling
authorAndy Lutomirski <luto@kernel.org>
Thu, 10 Mar 2016 03:00:30 +0000 (19:00 -0800)
committerIngo Molnar <mingo@kernel.org>
Thu, 10 Mar 2016 08:48:13 +0000 (09:48 +0100)
Due to a blatant design error, SYSENTER doesn't clear TF (single-step).

As a result, if a user does SYSENTER with TF set, we will single-step
through the kernel until something clears TF.  There is absolutely
nothing we can do to prevent this short of turning off SYSENTER [1].

Simplify the handling considerably with two changes:

  1. We already sanitize EFLAGS in SYSENTER to clear NT and AC.  We can
     add TF to that list of flags to sanitize with no overhead whatsoever.

  2. Teach do_debug() to ignore single-step traps in the SYSENTER prologue.

That's all we need to do.

Don't get too excited -- our handling is still buggy on 32-bit
kernels.  There's nothing wrong with the SYSENTER code itself, but
the #DB prologue has a clever fixup for traps on the very first
instruction of entry_SYSENTER_32, and the fixup doesn't work quite
correctly.  The next two patches will fix that.

[1] We could probably prevent it by forcing BTF on at all times and
    making sure we clear TF before any branches in the SYSENTER
    code.  Needless to say, this is a bad idea.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/a30d2ea06fe4b621fe6a9ef911b02c0f38feb6f2.1457578375.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
arch/x86/entry/entry_32.S
arch/x86/entry/entry_64_compat.S
arch/x86/include/asm/proto.h
arch/x86/kernel/traps.c

index 76109068149f6b52bf4cd59c3e0b44399081d843..b2e1d446bdf8bac9d871f51af2c150434afdc36e 100644 (file)
@@ -287,7 +287,26 @@ need_resched:
 END(resume_kernel)
 #endif
 
-       # SYSENTER  call handler stub
+GLOBAL(__begin_SYSENTER_singlestep_region)
+/*
+ * All code from here through __end_SYSENTER_singlestep_region is subject
+ * to being single-stepped if a user program sets TF and executes SYSENTER.
+ * There is absolutely nothing that we can do to prevent this from happening
+ * (thanks Intel!).  To keep our handling of this situation as simple as
+ * possible, we handle TF just like AC and NT, except that our #DB handler
+ * will ignore all of the single-step traps generated in this range.
+ */
+
+#ifdef CONFIG_XEN
+/*
+ * Xen doesn't set %esp to be precisely what the normal SYSENTER
+ * entry point expects, so fix it up before using the normal path.
+ */
+ENTRY(xen_sysenter_target)
+       addl    $5*4, %esp                      /* remove xen-provided frame */
+       jmp     sysenter_past_esp
+#endif
+
 ENTRY(entry_SYSENTER_32)
        movl    TSS_sysenter_sp0(%esp), %esp
 sysenter_past_esp:
@@ -301,19 +320,25 @@ sysenter_past_esp:
        SAVE_ALL pt_regs_ax=$-ENOSYS    /* save rest */
 
        /*
-        * SYSENTER doesn't filter flags, so we need to clear NT and AC
-        * ourselves.  To save a few cycles, we can check whether
+        * SYSENTER doesn't filter flags, so we need to clear NT, AC
+        * and TF ourselves.  To save a few cycles, we can check whether
         * either was set instead of doing an unconditional popfq.
         * This needs to happen before enabling interrupts so that
         * we don't get preempted with NT set.
         *
+        * If TF is set, we will single-step all the way to here -- do_debug
+        * will ignore all the traps.  (Yes, this is slow, but so is
+        * single-stepping in general.  This allows us to avoid having
+        * a more complicated code to handle the case where a user program
+        * forces us to single-step through the SYSENTER entry code.)
+        *
         * NB.: .Lsysenter_fix_flags is a label with the code under it moved
         * out-of-line as an optimization: NT is unlikely to be set in the
         * majority of the cases and instead of polluting the I$ unnecessarily,
         * we're keeping that code behind a branch which will predict as
         * not-taken and therefore its instructions won't be fetched.
         */
-       testl   $X86_EFLAGS_NT|X86_EFLAGS_AC, PT_EFLAGS(%esp)
+       testl   $X86_EFLAGS_NT|X86_EFLAGS_AC|X86_EFLAGS_TF, PT_EFLAGS(%esp)
        jnz     .Lsysenter_fix_flags
 .Lsysenter_flags_fixed:
 
@@ -369,6 +394,7 @@ sysenter_past_esp:
        pushl   $X86_EFLAGS_FIXED
        popfl
        jmp     .Lsysenter_flags_fixed
+GLOBAL(__end_SYSENTER_singlestep_region)
 ENDPROC(entry_SYSENTER_32)
 
        # system call handler stub
@@ -651,14 +677,6 @@ ENTRY(spurious_interrupt_bug)
 END(spurious_interrupt_bug)
 
 #ifdef CONFIG_XEN
-/*
- * Xen doesn't set %esp to be precisely what the normal SYSENTER
- * entry point expects, so fix it up before using the normal path.
- */
-ENTRY(xen_sysenter_target)
-       addl    $5*4, %esp                      /* remove xen-provided frame */
-       jmp     sysenter_past_esp
-
 ENTRY(xen_hypervisor_callback)
        pushl   $-1                             /* orig_ax = -1 => not a system call */
        SAVE_ALL
index da5c19443e0b51796ac145180f967868617ea165..cbb5e6943d9b696bb93753730a9803ad0b8a4a1c 100644 (file)
@@ -94,13 +94,19 @@ ENTRY(entry_SYSENTER_compat)
         * This needs to happen before enabling interrupts so that
         * we don't get preempted with NT set.
         *
+        * If TF is set, we will single-step all the way to here -- do_debug
+        * will ignore all the traps.  (Yes, this is slow, but so is
+        * single-stepping in general.  This allows us to avoid having
+        * a more complicated code to handle the case where a user program
+        * forces us to single-step through the SYSENTER entry code.)
+        *
         * NB.: .Lsysenter_fix_flags is a label with the code under it moved
         * out-of-line as an optimization: NT is unlikely to be set in the
         * majority of the cases and instead of polluting the I$ unnecessarily,
         * we're keeping that code behind a branch which will predict as
         * not-taken and therefore its instructions won't be fetched.
         */
-       testl   $X86_EFLAGS_NT|X86_EFLAGS_AC, EFLAGS(%rsp)
+       testl   $X86_EFLAGS_NT|X86_EFLAGS_AC|X86_EFLAGS_TF, EFLAGS(%rsp)
        jnz     .Lsysenter_fix_flags
 .Lsysenter_flags_fixed:
 
@@ -121,6 +127,7 @@ ENTRY(entry_SYSENTER_compat)
        pushq   $X86_EFLAGS_FIXED
        popfq
        jmp     .Lsysenter_flags_fixed
+GLOBAL(__end_entry_SYSENTER_compat)
 ENDPROC(entry_SYSENTER_compat)
 
 /*
index a4a77286cb1ddf68b466e934e9f69cb1785f3062..9b9b30b1944187c8c3de3ed5c4ef1d82ea7c422d 100644 (file)
@@ -7,12 +7,23 @@
 
 void syscall_init(void);
 
+#ifdef CONFIG_X86_64
 void entry_SYSCALL_64(void);
-void entry_SYSCALL_compat(void);
+#endif
+
+#ifdef CONFIG_X86_32
 void entry_INT80_32(void);
-void entry_INT80_compat(void);
 void entry_SYSENTER_32(void);
+void __begin_SYSENTER_singlestep_region(void);
+void __end_SYSENTER_singlestep_region(void);
+#endif
+
+#ifdef CONFIG_IA32_EMULATION
 void entry_SYSENTER_compat(void);
+void __end_entry_SYSENTER_compat(void);
+void entry_SYSCALL_compat(void);
+void entry_INT80_compat(void);
+#endif
 
 void x86_configure_nx(void);
 void x86_report_nx(void);
index 7394be8b1a669d71cd807ee91251fd4a628afe81..b0ddb81926f92bc7ecf64e862be3d35853fdadcf 100644 (file)
@@ -559,6 +559,29 @@ struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s)
 NOKPROBE_SYMBOL(fixup_bad_iret);
 #endif
 
+static bool is_sysenter_singlestep(struct pt_regs *regs)
+{
+       /*
+        * We don't try for precision here.  If we're anywhere in the region of
+        * code that can be single-stepped in the SYSENTER entry path, then
+        * assume that this is a useless single-step trap due to SYSENTER
+        * being invoked with TF set.  (We don't know in advance exactly
+        * which instructions will be hit because BTF could plausibly
+        * be set.)
+        */
+#ifdef CONFIG_X86_32
+       return (regs->ip - (unsigned long)__begin_SYSENTER_singlestep_region) <
+               (unsigned long)__end_SYSENTER_singlestep_region -
+               (unsigned long)__begin_SYSENTER_singlestep_region;
+#elif defined(CONFIG_IA32_EMULATION)
+       return (regs->ip - (unsigned long)entry_SYSENTER_compat) <
+               (unsigned long)__end_entry_SYSENTER_compat -
+               (unsigned long)entry_SYSENTER_compat;
+#else
+       return false;
+#endif
+}
+
 /*
  * Our handling of the processor debug registers is non-trivial.
  * We do not clear them on entry and exit from the kernel. Therefore
@@ -616,6 +639,18 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
         */
        clear_tsk_thread_flag(tsk, TIF_BLOCKSTEP);
 
+       if (unlikely(!user_mode(regs) && (dr6 & DR_STEP) &&
+                    is_sysenter_singlestep(regs))) {
+               dr6 &= ~DR_STEP;
+               if (!dr6)
+                       goto exit;
+               /*
+                * else we might have gotten a single-step trap and hit a
+                * watchpoint at the same time, in which case we should fall
+                * through and handle the watchpoint.
+                */
+       }
+
        /*
         * If dr6 has no reason to give us about the origin of this trap,
         * then it's very likely the result of an icebp/int01 trap.
@@ -624,7 +659,7 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
        if (!dr6 && user_mode(regs))
                user_icebp = 1;
 
-       /* Catch kmemcheck conditions first of all! */
+       /* Catch kmemcheck conditions! */
        if ((dr6 & DR_STEP) && kmemcheck_trap(regs))
                goto exit;
 
@@ -659,14 +694,13 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
                goto exit;
        }
 
-       /*
-        * Single-stepping through system calls: ignore any exceptions in
-        * kernel space, but re-enable TF when returning to user mode.
-        *
-        * We already checked v86 mode above, so we can check for kernel mode
-        * by just checking the CPL of CS.
-        */
-       if ((dr6 & DR_STEP) && !user_mode(regs)) {
+       if (WARN_ON_ONCE((dr6 & DR_STEP) && !user_mode(regs))) {
+               /*
+                * Historical junk that used to handle SYSENTER single-stepping.
+                * This should be unreachable now.  If we survive for a while
+                * without anyone hitting this warning, we'll turn this into
+                * an oops.
+                */
                tsk->thread.debugreg6 &= ~DR_STEP;
                set_tsk_thread_flag(tsk, TIF_SINGLESTEP);
                regs->flags &= ~X86_EFLAGS_TF;