[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen-unstable] [XEN] Fix the x86 emulator to safely fail when it turns out the



# HG changeset patch
# User kfraser@xxxxxxxxxxxxxxxxxxxxx
# Node ID 51a98a6c2c054bfc37c90a5a3f29929f2347bda8
# Parent  d389123fad85966deb081e169b368f04256516e2
[XEN] Fix the x86 emulator to safely fail when it turns out the
faulting memory access was to/from an implicit memory operand
(typically either an instruction fetch or stack access).
Rationalise use of macros for page fault error code flags.
This patch supercedes the fix in changeset 11242.
Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>
---
 tools/tests/test_x86_emulator.c     |   14 ++++++++++++++
 xen/arch/x86/shadow2.c              |   10 +++++-----
 xen/arch/x86/traps.c                |   16 ++++++++--------
 xen/arch/x86/x86_emulate.c          |   35 ++++++++++++++++++++++++++++++-----
 xen/include/asm-x86/processor.h     |   10 +++++-----
 xen/include/asm-x86/shadow2-types.h |   13 -------------
 6 files changed, 62 insertions(+), 36 deletions(-)

diff -r d389123fad85 -r 51a98a6c2c05 tools/tests/test_x86_emulator.c
--- a/tools/tests/test_x86_emulator.c   Wed Aug 23 17:25:11 2006 +0100
+++ b/tools/tests/test_x86_emulator.c   Wed Aug 23 18:38:08 2006 +0100
@@ -15,6 +15,8 @@ typedef int64_t            s64;
 #include <asm-x86/x86_emulate.h>
 #include <sys/mman.h>
 
+#define PFEC_write_access (1U<<1)
+
 static int read_any(
     unsigned long addr,
     unsigned long *val,
@@ -105,6 +107,7 @@ int main(int argc, char **argv)
     regs.eflags = 0x200;
     regs.eip    = (unsigned long)&instr[0];
     regs.ecx    = 0x12345678;
+    regs.error_code = PFEC_write_access;
     ctxt.cr2    = (unsigned long)res;
     *res        = 0x7FFFFFFF;
     rc = x86_emulate_memop(&ctxt, &emulops);
@@ -125,6 +128,7 @@ int main(int argc, char **argv)
     regs.ecx    = 0x12345678UL;
 #endif
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x92345677) || 
@@ -139,6 +143,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.ecx    = ~0UL;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x92345677) || 
@@ -154,6 +159,7 @@ int main(int argc, char **argv)
     regs.eax    = 0x92345677UL;
     regs.ecx    = 0xAA;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x923456AA) || 
@@ -170,6 +176,7 @@ int main(int argc, char **argv)
     regs.eax    = 0xAABBCC77UL;
     regs.ecx    = 0xFF;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x923456AA) || 
@@ -186,6 +193,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.ecx    = 0x12345678;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x12345678) || 
@@ -203,6 +211,7 @@ int main(int argc, char **argv)
     regs.eax    = 0x923456AAUL;
     regs.ecx    = 0xDDEEFF00L;
     ctxt.cr2    = (unsigned long)res;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0xDDEEFF00) || 
@@ -240,6 +249,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.edi    = (unsigned long)res;
     ctxt.cr2    = regs.edi;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (*res != 0x2233445D) ||
@@ -261,6 +271,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.edi    = (unsigned long)res;
     ctxt.cr2    = regs.edi;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (res[0] != 0x9999AAAA) ||
@@ -275,6 +286,7 @@ int main(int argc, char **argv)
     regs.eip    = (unsigned long)&instr[0];
     regs.edi    = (unsigned long)res;
     ctxt.cr2    = regs.edi;
+    regs.error_code = PFEC_write_access;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) || 
          (res[0] != 0x9999AAAA) ||
@@ -292,6 +304,7 @@ int main(int argc, char **argv)
     regs.ecx    = 0x12345678;
     ctxt.cr2    = (unsigned long)res;
     *res        = 0x82;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) ||
          (*res != 0x82) ||
@@ -307,6 +320,7 @@ int main(int argc, char **argv)
     regs.ecx    = 0x12345678;
     ctxt.cr2    = (unsigned long)res;
     *res        = 0x1234aa82;
+    regs.error_code = 0;
     rc = x86_emulate_memop(&ctxt, &emulops);
     if ( (rc != 0) ||
          (*res != 0x1234aa82) ||
diff -r d389123fad85 -r 51a98a6c2c05 xen/arch/x86/shadow2.c
--- a/xen/arch/x86/shadow2.c    Wed Aug 23 17:25:11 2006 +0100
+++ b/xen/arch/x86/shadow2.c    Wed Aug 23 18:38:08 2006 +0100
@@ -2893,7 +2893,7 @@ static int sh2_page_fault(struct vcpu *v
     // i.e. ring 3.  Such errors are not caused or dealt with by the shadow
     // code.
     //
-    if ( (regs->error_code & X86_PFEC_SUPERVISOR_FAULT) &&
+    if ( (regs->error_code & PFEC_user_mode) &&
          !(accumulated_gflags & _PAGE_USER) )
     {
         /* illegal user-mode access to supervisor-only page */
@@ -2903,7 +2903,7 @@ static int sh2_page_fault(struct vcpu *v
 
     // Was it a write fault?
     //
-    if ( regs->error_code & X86_PFEC_WRITE_FAULT )
+    if ( regs->error_code & PFEC_write_access )
     {
         if ( unlikely(!(accumulated_gflags & _PAGE_RW)) )
         {
@@ -2917,7 +2917,7 @@ static int sh2_page_fault(struct vcpu *v
         // marked "do not execute".  Such errors are not caused or dealt with
         // by the shadow code.
         //
-        if ( regs->error_code & X86_PFEC_INSN_FETCH_FAULT )
+        if ( regs->error_code & PFEC_insn_fetch )
         {
             if ( accumulated_gflags & _PAGE_NX_BIT )
             {
@@ -2960,8 +2960,8 @@ static int sh2_page_fault(struct vcpu *v
      * for the shadow entry, since we might promote a page here. */
     // XXX -- this code will need to change somewhat if/when the shadow code
     // can directly map superpages...
-    ft = ((regs->error_code & X86_PFEC_WRITE_FAULT) 
-          ? ft_demand_write : ft_demand_read);
+    ft = ((regs->error_code & PFEC_write_access) ?
+          ft_demand_write : ft_demand_read);
     ptr_sl1e = shadow_get_and_create_l1e(v, &gw, &sl1mfn, ft);
     ASSERT(ptr_sl1e);
 
diff -r d389123fad85 -r 51a98a6c2c05 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c      Wed Aug 23 17:25:11 2006 +0100
+++ b/xen/arch/x86/traps.c      Wed Aug 23 18:38:08 2006 +0100
@@ -686,9 +686,9 @@ void propagate_page_fault(unsigned long 
     v->vcpu_info->arch.cr2           = addr;
 
     /* Re-set error_code.user flag appropriately for the guest. */
-    error_code &= ~PGERR_user_mode;
+    error_code &= ~PFEC_user_mode;
     if ( !guest_kernel_mode(v, guest_cpu_user_regs()) )
-        error_code |= PGERR_user_mode;
+        error_code |= PFEC_user_mode;
 
     ti = &v->arch.guest_context.trap_ctxt[TRAP_page_fault];
     tb->flags = TBF_EXCEPTION | TBF_EXCEPTION_ERRCODE;
@@ -768,17 +768,17 @@ static int __spurious_page_fault(
     unsigned int required_flags, disallowed_flags;
 
     /* Reserved bit violations are never spurious faults. */
-    if ( regs->error_code & PGERR_reserved_bit )
+    if ( regs->error_code & PFEC_reserved_bit )
         return 0;
 
     required_flags  = _PAGE_PRESENT;
-    if ( regs->error_code & PGERR_write_access )
+    if ( regs->error_code & PFEC_write_access )
         required_flags |= _PAGE_RW;
-    if ( regs->error_code & PGERR_user_mode )
+    if ( regs->error_code & PFEC_user_mode )
         required_flags |= _PAGE_USER;
 
     disallowed_flags = 0;
-    if ( regs->error_code & PGERR_instr_fetch )
+    if ( regs->error_code & PFEC_insn_fetch )
         disallowed_flags |= _PAGE_NX;
 
     mfn = cr3 >> PAGE_SHIFT;
@@ -886,7 +886,7 @@ static int fixup_page_fault(unsigned lon
          guest_kernel_mode(v, regs) &&
          /* Do not check if access-protection fault since the page may 
             legitimately be not present in shadow page tables */
-         ((regs->error_code & PGERR_write_access) == PGERR_write_access) &&
+         ((regs->error_code & PFEC_write_access) == PFEC_write_access) &&
          ptwr_do_page_fault(d, addr, regs) )
         return EXCRET_fault_fixed;
 
@@ -1100,7 +1100,7 @@ static int emulate_privileged_op(struct 
             if ( (rc = copy_to_user((void *)regs->edi, &data, op_bytes)) != 0 )
             {
                 propagate_page_fault(regs->edi + op_bytes - rc,
-                                     PGERR_write_access);
+                                     PFEC_write_access);
                 return EXCRET_fault_fixed;
             }
             regs->edi += (int)((regs->eflags & EF_DF) ? -op_bytes : op_bytes);
diff -r d389123fad85 -r 51a98a6c2c05 xen/arch/x86/x86_emulate.c
--- a/xen/arch/x86/x86_emulate.c        Wed Aug 23 17:25:11 2006 +0100
+++ b/xen/arch/x86/x86_emulate.c        Wed Aug 23 18:38:08 2006 +0100
@@ -20,6 +20,11 @@
 #define DPRINTF DPRINTK
 #endif
 #include <asm-x86/x86_emulate.h>
+
+#ifndef PFEC_write_access
+#define PFEC_write_access (1U<<1)
+#define PFEC_insn_fetch   (1U<<4)
+#endif
 
 /*
  * Opcode effective-address decode tables.
@@ -444,6 +449,13 @@ x86_emulate_memop(
     /* Shadow copy of register state. Committed on successful emulation. */
     struct cpu_user_regs _regs = *ctxt->regs;
 
+    /*
+     * We do not emulate faults on instruction fetch. We assume that the
+     * guest never executes out of a special memory area.
+     */
+    if ( _regs.error_code & PFEC_insn_fetch )
+        return -1;
+
     switch ( mode )
     {
     case X86EMUL_MODE_REAL:
@@ -620,6 +632,14 @@ x86_emulate_memop(
         }
         break;
     case DstMem:
+        /*
+         * We expect that the fault occurred while accessing the explicit
+         * destination memory operand. This is clearly not the case if the
+         * fault occurred on a read access (eg. POP has an *implicit* operand
+         * but we expect that the guest never uses special memory as stack).
+         */
+        if ( !(_regs.error_code & PFEC_write_access) )
+            goto cannot_emulate;
         dst.type  = OP_MEM;
         dst.ptr   = (unsigned long *)cr2;
         dst.bytes = (d & ByteOp) ? 1 : op_bytes;
@@ -664,6 +684,14 @@ x86_emulate_memop(
     case SrcMem:
         src.bytes = (d & ByteOp) ? 1 : op_bytes;
     srcmem_common:
+        /*
+         * We expect that the fault occurred while accessing the explicit
+         * source memory operand. This is clearly not the case if the fault
+         * occurred on a write access (eg. PUSH has an *implicit* operand
+         * but we expect that the guest never uses special memory as stack).
+         */
+        if ( _regs.error_code & PFEC_write_access )
+            goto cannot_emulate;
         src.type  = OP_MEM;
         src.ptr   = (unsigned long *)cr2;
         if ( (rc = ops->read_emulated((unsigned long)src.ptr, 
@@ -846,9 +874,6 @@ x86_emulate_memop(
             emulate_1op("dec", dst, _regs.eflags);
             break;
         case 6: /* push */
-            /* Don't emulate if fault was on stack */
-            if ( _regs.error_code & 2 )
-                goto cannot_emulate; 
             /* 64-bit mode: PUSH always pushes a 64-bit operand. */
             if ( mode == X86EMUL_MODE_PROT64 )
             {
@@ -923,7 +948,7 @@ x86_emulate_memop(
     case 0xa4 ... 0xa5: /* movs */
         dst.type  = OP_MEM;
         dst.bytes = (d & ByteOp) ? 1 : op_bytes;
-        if ( _regs.error_code & 2 )
+        if ( _regs.error_code & PFEC_write_access )
         {
             /* Write fault: destination is special memory. */
             dst.ptr = (unsigned long *)cr2;
@@ -1165,7 +1190,7 @@ x86_emulate_write_std(
 
     if ( (rc = copy_to_user((void *)addr, (void *)&val, bytes)) != 0 )
     {
-        propagate_page_fault(addr + bytes - rc, PGERR_write_access);
+        propagate_page_fault(addr + bytes - rc, PFEC_write_access);
         return X86EMUL_PROPAGATE_FAULT;
     }
 
diff -r d389123fad85 -r 51a98a6c2c05 xen/include/asm-x86/processor.h
--- a/xen/include/asm-x86/processor.h   Wed Aug 23 17:25:11 2006 +0100
+++ b/xen/include/asm-x86/processor.h   Wed Aug 23 18:38:08 2006 +0100
@@ -130,11 +130,11 @@
 #define TF_kernel_mode         (1<<_TF_kernel_mode)
 
 /* #PF error code values. */
-#define PGERR_page_present   (1U<<0)
-#define PGERR_write_access   (1U<<1)
-#define PGERR_user_mode      (1U<<2)
-#define PGERR_reserved_bit   (1U<<3)
-#define PGERR_instr_fetch    (1U<<4)
+#define PFEC_page_present   (1U<<0)
+#define PFEC_write_access   (1U<<1)
+#define PFEC_user_mode      (1U<<2)
+#define PFEC_reserved_bit   (1U<<3)
+#define PFEC_insn_fetch     (1U<<4)
 
 #ifndef __ASSEMBLY__
 
diff -r d389123fad85 -r 51a98a6c2c05 xen/include/asm-x86/shadow2-types.h
--- a/xen/include/asm-x86/shadow2-types.h       Wed Aug 23 17:25:11 2006 +0100
+++ b/xen/include/asm-x86/shadow2-types.h       Wed Aug 23 18:38:08 2006 +0100
@@ -460,19 +460,6 @@ struct shadow2_walk_t
     mfn_t l2mfn;                /* MFN that the level 2 entry is in */
     mfn_t l1mfn;                /* MFN that the level 1 entry is in */
 };
-
-
-/* X86 error code bits:
- * These bits certainly ought to be defined somewhere other than here,
- * but until that place is determined, here they sit.
- *
- * "PFEC" == "Page Fault Error Code"
- */
-#define X86_PFEC_PRESENT            1  /* 0 == page was not present */
-#define X86_PFEC_WRITE_FAULT        2  /* 0 == reading, 1 == writing */
-#define X86_PFEC_SUPERVISOR_FAULT   4  /* 0 == supervisor-mode, 1 == user */
-#define X86_PFEC_RESERVED_BIT_FAULT 8  /* 1 == reserved bits set in pte */
-#define X86_PFEC_INSN_FETCH_FAULT  16  /* 0 == normal, 1 == instr'n fetch */
 
 /* macros for dealing with the naming of the internal function names of the
  * shadow code's external entry points.

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.