[PATCH] x86/vmx: Fix injection of #DB traps following XSA-156

[PATCH] x86/vmx: Fix injection of #DB traps following XSA-156

[Andrew Cooper]

Most #DB exceptions are traps rather than faults, meaning that the instruction
pointer in the exception frame points after the instruction rather than at it.

However, VMX intercepts all have fault semantics, even when intercepting a
trap.  Re-injecting an intercepted trap as a fault causes an infinite loop in
the guest, by re-executing the same trapping instruction repeatedly.  This
breaks debugging inside the guest.

Introduce a helper which copies VM_EXIT_INTR_INTO to VM_ENTRY_INTR_INFO, and
use it to mirror the intercepted interrupt back to the guest.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: security@xen.org

v2: Drop vmx_intr_info_t to make the patch simpler to backport.
---
 xen/arch/x86/hvm/vmx/vmx.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b918b8a..7917fb7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2877,6 +2877,33 @@ static int vmx_handle_eoi_write(void)
     return 0;
 }
 
+/*
+ * Propagate VM_EXIT_INTR_INFO to VM_ENTRY_INTR_INFO.  Used to mirror an
+ * intercepted exception back to the guest as if Xen hadn't intercepted it.
+ *
+ * It is the callers responsibility to ensure that this function is only used
+ * in the context of an appropriate vmexit.
+ */
+static void vmx_propagate_intr(void)
+{
+    unsigned long intr, tmp;
+
+    __vmread(VM_EXIT_INTR_INFO, &intr);
+
+    ASSERT(intr & INTR_INFO_VALID_MASK);
+
+    __vmwrite(VM_ENTRY_INTR_INFO, intr);
+
+    if ( intr & INTR_INFO_DELIVER_CODE_MASK )
+    {
+        __vmread(VM_EXIT_INTR_ERROR_CODE, &tmp);
+        __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, tmp);
+    }
+
+    __vmread(VM_EXIT_INSTRUCTION_LEN, &tmp);
+    __vmwrite(VM_ENTRY_INSTRUCTION_LEN, tmp);
+}
+
 static void vmx_idtv_reinject(unsigned long idtv_info)
 {
 
@@ -3137,7 +3164,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
             HVMTRACE_1D(TRAP_DEBUG, exit_qualification);
             write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE);
             if ( !v->domain->debugger_attached )
-                hvm_inject_hw_exception(vector, HVM_DELIVER_NO_ERROR_CODE);
+                vmx_propagate_intr();
             else
                 domain_pause_for_debugger();
             break;
@@ -3206,8 +3233,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
             break;
         case TRAP_alignment_check:
             HVMTRACE_1D(TRAP, vector);
-            __vmread(VM_EXIT_INTR_ERROR_CODE, &ecode);
-            hvm_inject_hw_exception(vector, ecode);
+            vmx_propagate_intr();
             break;
         case TRAP_nmi:
             if ( MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) !=
-- 
2.1.4

Re: [PATCH] x86/vmx: Fix injection of #DB traps following XSA-156

[Tian, Kevin]

> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Monday, January 04, 2016 6:00 PM
> 
> Most #DB exceptions are traps rather than faults, meaning that the instruction
> pointer in the exception frame points after the instruction rather than at it.
> 
> However, VMX intercepts all have fault semantics, even when intercepting a
> trap.  Re-injecting an intercepted trap as a fault causes an infinite loop in
> the guest, by re-executing the same trapping instruction repeatedly.  This
> breaks debugging inside the guest.
> 
> Introduce a helper which copies VM_EXIT_INTR_INTO to VM_ENTRY_INTR_INFO, and
> use it to mirror the intercepted interrupt back to the guest.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>

Re: [PATCH] x86/vmx: Fix injection of #DB traps following XSA-156

[Ian Campbell]

On Tue, 2016-01-05 at 06:53 +0000, Tian, Kevin wrote:
> > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> > Sent: Monday, January 04, 2016 6:00 PM
> > 
> > Most #DB exceptions are traps rather than faults, meaning that the
> > instruction
> > pointer in the exception frame points after the instruction rather than
> > at it.
> > 
> > However, VMX intercepts all have fault semantics, even when
> > intercepting a
> > trap.  Re-injecting an intercepted trap as a fault causes an infinite
> > loop in
> > the guest, by re-executing the same trapping instruction
> > repeatedly.  This
> > breaks debugging inside the guest.
> > 
> > Introduce a helper which copies VM_EXIT_INTR_INTO to
> > VM_ENTRY_INTR_INFO, and
> > use it to mirror the intercepted interrupt back to the guest.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Acked-by: Kevin Tian <kevin.tian@intel.com>

Thanks. Andy tells me this issue is causing quite some havoc in the field
and we aren't sure when Jan is back from vacation so I have stepped outside
the usual scope of things I commit and applied this one. I hope that's OK
with everyone.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel