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

[Xen-changelog] [xen stable-4.3] x86: limit checks in hypercall_xlat_continuation() to actual arguments



commit 5e687e7da367827344db3965b8a5f5f761a8431a
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Thu Nov 27 14:14:15 2014 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Nov 27 14:14:15 2014 +0100

    x86: limit checks in hypercall_xlat_continuation() to actual arguments
    
    HVM/PVH guests can otherwise trigger the final BUG_ON() in that
    function by entering 64-bit mode, setting the high halves of affected
    registers to non-zero values, leaving 64-bit mode, and issuing a
    hypercall that might get preempted and hence become subject to
    continuation argument translation (HYPERVISOR_memory_op being the only
    one possible for HVM, PVH also having the option of using
    HYPERVISOR_mmuext_op). This issue got introduced when HVM code was
    switched to use compat_memory_op() - neither that nor
    hypercall_xlat_continuation() were originally intended to be used by
    other than PV guests (which can't enter 64-bit mode and hence have no
    way to alter the high halves of 64-bit registers).
    
    This is CVE-2014-8866 / XSA-111.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Tim Deegan <tim@xxxxxxx>
---
 xen/arch/x86/domain.c           |   12 ++++++++----
 xen/arch/x86/x86_64/compat/mm.c |    8 ++++----
 xen/common/compat/memory.c      |    2 +-
 xen/include/xen/compat.h        |    5 ++++-
 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 336cd79..74e1146 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1653,7 +1653,8 @@ unsigned long hypercall_create_continuation(
     return op;
 }
 
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...)
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+                                unsigned int mask, ...)
 {
     int rc = 0;
     struct mc_state *mcs = &current->mc_state;
@@ -1662,7 +1663,10 @@ int hypercall_xlat_continuation(unsigned int *id, 
unsigned int mask, ...)
     unsigned long nval = 0;
     va_list args;
 
-    BUG_ON(id && *id > 5);
+    ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
+    ASSERT(!(mask >> nr));
+
+    BUG_ON(id && *id >= nr);
     BUG_ON(id && (mask & (1U << *id)));
 
     va_start(args, mask);
@@ -1671,7 +1675,7 @@ int hypercall_xlat_continuation(unsigned int *id, 
unsigned int mask, ...)
     {
         if ( !test_bit(_MCSF_call_preempted, &mcs->flags) )
             return 0;
-        for ( i = 0; i < 6; ++i, mask >>= 1 )
+        for ( i = 0; i < nr; ++i, mask >>= 1 )
         {
             if ( mask & 1 )
             {
@@ -1699,7 +1703,7 @@ int hypercall_xlat_continuation(unsigned int *id, 
unsigned int mask, ...)
     else
     {
         regs = guest_cpu_user_regs();
-        for ( i = 0; i < 6; ++i, mask >>= 1 )
+        for ( i = 0; i < nr; ++i, mask >>= 1 )
         {
             unsigned long *reg;
 
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index f01f8ff..f1e63d6 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -78,7 +78,7 @@ int compat_arch_memory_op(int op, 
XEN_GUEST_HANDLE_PARAM(void) arg)
         }
 
         if ( rc == __HYPERVISOR_memory_op )
-            hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+            hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
 
         break;
     }
@@ -144,7 +144,7 @@ int compat_arch_memory_op(int op, 
XEN_GUEST_HANDLE_PARAM(void) arg)
             break;
 
         if ( rc == __HYPERVISOR_memory_op )
-            hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+            hypercall_xlat_continuation(NULL, 2, 0x2, nat, arg);
 
         XLAT_pod_target(&cmp, nat);
 
@@ -379,7 +379,7 @@ int 
compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
                 left = 1;
                 if ( arg1 != MMU_UPDATE_PREEMPTED )
                 {
-                    BUG_ON(!hypercall_xlat_continuation(&left, 0x01, nat_ops,
+                    BUG_ON(!hypercall_xlat_continuation(&left, 4, 0x01, 
nat_ops,
                                                         cmp_uops));
                     if ( !test_bit(_MCSF_in_multicall, &mcs->flags) )
                         regs->_ecx += count - i;
@@ -387,7 +387,7 @@ int 
compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
                         mcs->compat_call.args[1] += count - i;
                 }
                 else
-                    BUG_ON(hypercall_xlat_continuation(&left, 0));
+                    BUG_ON(hypercall_xlat_continuation(&left, 4, 0));
                 BUG_ON(left != arg1);
             }
             else
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index ba7e6eb..04ba0db 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -208,7 +208,7 @@ int compat_memory_op(unsigned int cmd, 
XEN_GUEST_HANDLE_PARAM(void) compat)
             break;
 
         cmd = 0;
-        if ( hypercall_xlat_continuation(&cmd, 0x02, nat.hnd, compat) )
+        if ( hypercall_xlat_continuation(&cmd, 2, 0x02, nat.hnd, compat) )
         {
             BUG_ON(rc != __HYPERVISOR_memory_op);
             BUG_ON((cmd & MEMOP_CMD_MASK) != op);
diff --git a/xen/include/xen/compat.h b/xen/include/xen/compat.h
index ca60699..bb3ffd1 100644
--- a/xen/include/xen/compat.h
+++ b/xen/include/xen/compat.h
@@ -195,6 +195,8 @@ static inline int name(k xen_ ## n *x, k compat_ ## n *c) \
  * This option is useful for extracting the "op" argument or similar from the
  * hypercall to enable further xlat processing.
  *
+ * nr: Total number of arguments the hypercall has.
+ *
  * mask: Specifies which of the hypercall arguments require compat translation.
  * bit 0 indicates that the 0'th argument requires translation, bit 1 indicates
  * that the first argument requires translation and so on. Native and compat
@@ -214,7 +216,8 @@ static inline int name(k xen_ ## n *x, k compat_ ## n *c) \
  *
  * Return: Number of arguments which were actually translated.
  */
-int hypercall_xlat_continuation(unsigned int *id, unsigned int mask, ...);
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+                                unsigned int mask, ...);
 
 /* In-place translation functons: */
 struct start_info;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.3

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
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®.