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

[Xen-changelog] [xen master] xen/mm: make sure node is less than MAX_NUMNODES



commit 2fece35303529395bfea6b03d2268380ef682c93
Author:     George Dunlap <george.dunlap@xxxxxxxxxx>
AuthorDate: Tue Sep 12 14:43:16 2017 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Sep 12 14:43:16 2017 +0200

    xen/mm: make sure node is less than MAX_NUMNODES
    
    The output of MEMF_get_node(memflags) can be as large as nodeid_t can
    hold (currently 255).  This is then used as an index to arrays of size
    MAX_NUMNODE, which is 64 on x86 and 1 on ARM, can be passed in by an
    untrusted guest (via memory_exchange and increase_reservation) and is
    not currently bounds-checked.
    
    Check the value in page_alloc.c before using it, and also check the
    value in the hypercall call sites and return -EINVAL if appropriate.
    Don't permit domains other than the hardware or control domain to
    allocate node-constrained memory.
    
    This is CVE-2017-14316 / XSA-231.
    
    Reported-by: Matthew Daley <mattd@xxxxxxxxxxx>
    Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/common/memory.c     | 40 +++++++++++++++++++++++++++++++++-------
 xen/common/page_alloc.c |  7 +++++--
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 26da605..a2abf55 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -429,6 +429,31 @@ static void decrease_reservation(struct memop_args *a)
     a->nr_done = i;
 }
 
+static bool propagate_node(unsigned int xmf, unsigned int *memflags)
+{
+    const struct domain *currd = current->domain;
+
+    BUILD_BUG_ON(XENMEMF_get_node(0) != NUMA_NO_NODE);
+    BUILD_BUG_ON(MEMF_get_node(0) != NUMA_NO_NODE);
+
+    if ( XENMEMF_get_node(xmf) == NUMA_NO_NODE )
+        return true;
+
+    if ( is_hardware_domain(currd) || is_control_domain(currd) )
+    {
+        if ( XENMEMF_get_node(xmf) >= MAX_NUMNODES )
+            return false;
+
+        *memflags |= MEMF_node(XENMEMF_get_node(xmf));
+        if ( xmf & XENMEMF_exact_node_request )
+            *memflags |= MEMF_exact_node;
+    }
+    else if ( xmf & XENMEMF_exact_node_request )
+        return false;
+
+    return true;
+}
+
 static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 {
     struct xen_memory_exchange exch;
@@ -501,6 +526,12 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
         }
     }
 
+    if ( unlikely(!propagate_node(exch.out.mem_flags, &memflags)) )
+    {
+        rc = -EINVAL;
+        goto fail_early;
+    }
+
     d = rcu_lock_domain_by_any_id(exch.in.domid);
     if ( d == NULL )
     {
@@ -519,7 +550,6 @@ static long 
memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
         d,
         XENMEMF_get_address_bits(exch.out.mem_flags) ? :
         (BITS_PER_LONG+PAGE_SHIFT)));
-    memflags |= MEMF_node(XENMEMF_get_node(exch.out.mem_flags));
 
     for ( i = (exch.nr_exchanged >> in_chunk_order);
           i < (exch.in.nr_extents >> in_chunk_order);
@@ -882,12 +912,8 @@ static int construct_memop_from_reservation(
         }
         read_unlock(&d->vnuma_rwlock);
     }
-    else
-    {
-        a->memflags |= MEMF_node(XENMEMF_get_node(r->mem_flags));
-        if ( r->mem_flags & XENMEMF_exact_node_request )
-            a->memflags |= MEMF_exact_node;
-    }
+    else if ( unlikely(!propagate_node(r->mem_flags, &a->memflags)) )
+        return -EINVAL;
 
     return 0;
 }
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index b5243fc..86c0794 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -781,10 +781,13 @@ static struct page_info *get_free_buddy(unsigned int 
zone_lo,
         if ( node >= MAX_NUMNODES )
             node = cpu_to_node(smp_processor_id());
     }
+    else if ( unlikely(node >= MAX_NUMNODES) )
+    {
+        ASSERT_UNREACHABLE();
+        return NULL;
+    }
     first_node = node;
 
-    ASSERT(node < MAX_NUMNODES);
-
     /*
      * Start with requested node, but exhaust all node memory in requested 
      * zone before failing, only calc new node value if we fail to find memory 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

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