x86/emul: Correct the handling of eflags with SYSCALL

A singlestep #DB is determined by the resulting eflags value from the
execution of SYSCALL, not the original eflags value.

By using the original eflags value, we negate the guest kernels attempt to
protect itself from a privilege escalation by masking TF.

(re)introduce a singlestep boolean, defaulting to the original eflags state,
but have the SYSCALL emulation recalculate it after masking has occurred.

This is XSA-204

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index f69decee10e94fd6b89c3644e71e5081954438c2..165eebb0d9646967bb9379f5847c29d598d20717 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2502,6 +2502,7 @@ x86_emulate(
     struct x86_emulate_state state;
     int rc;
     uint8_t b, d;
+    bool singlestep = ctxt->regs->eflags & EFLG_TF;
     struct operand src = { .reg = PTR_POISON };
     struct operand dst = { .reg = PTR_POISON };
     enum x86_swint_type swint_type;
@@ -4678,6 +4679,23 @@ x86_emulate(
              (rc = ops->write_segment(x86_seg_ss, &sreg, ctxt)) )
             goto done;
 
+        /*
+         * SYSCALL (unlike most instructions) evaluates its singlestep action
+         * based on the resulting EFLG_TF, not the starting EFLG_TF.
+         *
+         * As the #DB is raised after the CPL change and before the OS can
+         * switch stack, it is a large risk for privilege escalation.
+         *
+         * 64bit kernels should mask EFLG_TF in MSR_FMASK to avoid any
+         * vulnerability.  Running the #DB handler on an IST stack is also a
+         * mitigation.
+         *
+         * 32bit kernels have no ability to mask EFLG_TF at all.  Their only
+         * mitigation is to use a task gate for handling #DB (or to not use
+         * enable EFER.SCE to start with).
+         */
+        singlestep = _regs.eflags & EFLG_TF;
+
         break;
     }
 
@@ -5580,9 +5598,9 @@ x86_emulate(
     if ( !mode_64bit() )
         _regs.eip = (uint32_t)_regs.eip;
 
-    /* Was singestepping active at the start of this instruction? */
-    if ( (rc == X86EMUL_OKAY) && (ctxt->regs->eflags & EFLG_TF) )
-        ctxt->retire.singlestep = true;
+    /* Should a singlestep #DB be raised? */
+    if ( rc == X86EMUL_OKAY )
+        ctxt->retire.singlestep = singlestep;
 
     if ( rc != X86EMUL_DONE )
         *ctxt->regs = _regs;