xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Xen Security Advisory 231 (CVE-2017-14316) - Missing NUMA node parameter verification
@ 2017-09-12 12:03 Xen.org security team
  0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2017-09-12 12:03 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2017-14316 / XSA-231
                               version 3

               Missing NUMA node parameter verification

UPDATES IN VERSION 3
====================

Updated metadata file

Public release.

ISSUE DESCRIPTION
=================

The function `alloc_heap_pages` allows callers to specify the first
NUMA node that should be used for allocations through the `memflags`
parameter; the node is extracted using the `MEMF_get_node` macro.

While the function checks to see if the special constant
`NUMA_NO_NODE` is specified, it otherwise does not handle the case
where `node >= MAX_NUMNODES`.  This allows an out-of-bounds access
to an internal array.

IMPACT
======

An attacker using crafted hypercalls can execute arbitrary code within
Xen.

VULNERABLE SYSTEMS
==================

All versions of Xen are affected.

Both ARM and x86 are affected.

Both systems running HVM guests and system running PV guests are
affected.

MITIGATION
==========

No known mitigation.

CREDITS
=======

This issue was discovered by Matthew Daley.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

xsa231.patch           xen-unstable
xsa231-4.9.patch       Xen 4.9, Xen 4.8
xsa231-4.7.patch       Xen 4.7, Xen 4.6
xsa231-4.5.patch       Xen 4.5

$ sha256sum xsa231*
4255d2bc4ca668e7abcbf8256b0a8f21acef2a47a06d626aad6d22c685034587  xsa231.meta
b72af3fb8c44925ea7973533e8a8701becfc194f3e1c97f12af0392e1edd16a3  xsa231.patch
d9853b2d2649679d8810bd7e93f7b51bd9fefb3472da60ae464bde88aae3389c  xsa231-4.5.patch
ce29b56a0480f4835b37835b351e704d204bb0ccd22325f487127aa2776cc2cf  xsa231-4.7.patch
71a53a5133c8d4e381dd0e3e54205d31dea545ab62b261084dd3aea140f88cad  xsa231-4.9.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.


(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJZt80DAAoJEIP+FMlX6CvZrooIALgotDR4DC367J1SF87V2dHW
Wo2O05rF8uBl12ofMA4LirjPfbNq49ZikaDr01jq+srFZLDw72IzgjbNJOwThkZt
DHFR12LABvAPHT/Je58vGqS24HKKhK1o+Q0vDcbZHzBGXkj6gwxNC+DJAzF9D9Ye
qXtZv4GmkmhFs0nQuzUF8bLu7ZvIQjB7QVoXnOvynx/mpCI9GPvoRGLptIJhbc8A
CqSLsgF+7cXC6E8u/pp9XorpsQf2ekQwJMkLiG3UXieeShwrmY1mCE/vWBgsFeyj
k7/+dQhj6X+7vwLA385Df3cF7hDjDi23AJMUN1AuVd9fx9/ie4o+9nJIa0FpUOA=
=al8X
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa231.meta --]
[-- Type: application/octet-stream, Size: 1778 bytes --]

{
  "XSA": 231,
  "SupportedVersions": [
    "master",
    "4.9",
    "4.8",
    "4.7",
    "4.6",
    "4.5"
  ],
  "Trees": [
    "xen"
  ],
  "Recipes": {
    "4.5": {
      "XenVersion": "4.5",
      "Recipes": {
        "xen": {
          "StableRef": "3217129eb65c0d4995ed08fb8919e3c334cad548",
          "Prereqs": [
            226,
            227,
            230
          ],
          "Patches": [
            "xsa231-4.5.patch"
          ]
        }
      }
    },
    "4.6": {
      "XenVersion": "4.6",
      "Recipes": {
        "xen": {
          "StableRef": "b4660b4d4a35edac715c003c84326de2b0fa4f47",
          "Prereqs": [],
          "Patches": [
            "xsa231-4.7.patch"
          ]
        }
      }
    },
    "4.7": {
      "XenVersion": "4.7",
      "Recipes": {
        "xen": {
          "StableRef": "5151257626155d6e331cc9e66d896c84db1611e1",
          "Prereqs": [],
          "Patches": [
            "xsa231-4.7.patch"
          ]
        }
      }
    },
    "4.8": {
      "XenVersion": "4.8",
      "Recipes": {
        "xen": {
          "StableRef": "f5211ce75821e0f2cc55effd28dfbe908226970f",
          "Prereqs": [],
          "Patches": [
            "xsa231-4.9.patch"
          ]
        }
      }
    },
    "4.9": {
      "XenVersion": "4.9",
      "Recipes": {
        "xen": {
          "StableRef": "9bf14bbf990843bfec16a5d69d36cf46c7593d88",
          "Prereqs": [],
          "Patches": [
            "xsa231-4.9.patch"
          ]
        }
      }
    },
    "master": {
      "XenVersion": "master",
      "Recipes": {
        "xen": {
          "StableRef": "9053a74c08fd6abf43bb45ff932b4386de7e8510",
          "Prereqs": [],
          "Patches": [
            "xsa231.patch"
          ]
        }
      }
    }
  }
}

[-- Attachment #3: xsa231.patch --]
[-- Type: application/octet-stream, Size: 3635 bytes --]

From: George Dunlap <george.dunlap@citrix.com>
Subject: 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 XSA-231.

Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v4: Ignore physical node input for other than hardware and control
    domains.
v3: Drop printk().
v2: Properly deal with NUMA_NO_NODE being valid as input, but larger
    than MAX_NUMNODES. Drop trailing white space.

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -429,6 +429,31 @@ static void decrease_reservation(struct
     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_HA
         }
     }
 
+    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_HA
         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_reservat
         }
         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;
 }
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -776,9 +776,12 @@ static struct page_info *alloc_heap_page
         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 

[-- Attachment #4: xsa231-4.5.patch --]
[-- Type: application/octet-stream, Size: 3437 bytes --]

From: George Dunlap <george.dunlap@citrix.com>
Subject: 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 XSA-231.

Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -383,6 +383,30 @@ static void decrease_reservation(struct
     a->nr_done = i;
 }
 
+static bool_t propagate_node(unsigned int xmf, unsigned int *memflags)
+{
+    const struct domain *currd = current->domain;
+
+    BUILD_BUG_ON(XENMEMF_get_node(0) != NUMA_NO_NODE);
+
+    if ( XENMEMF_get_node(xmf) == NUMA_NO_NODE )
+        return 1;
+
+    if ( is_hardware_domain(currd) || is_control_domain(currd) )
+    {
+        if ( XENMEMF_get_node(xmf) >= MAX_NUMNODES )
+            return 0;
+
+        *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 0;
+
+    return 1;
+}
+
 static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 {
     struct xen_memory_exchange exch;
@@ -455,6 +479,12 @@ static long memory_exchange(XEN_GUEST_HA
         }
     }
 
+    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 )
     {
@@ -473,7 +503,6 @@ static long memory_exchange(XEN_GUEST_HA
         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);
@@ -814,9 +843,8 @@ long do_memory_op(unsigned long cmd, XEN
             args.memflags = MEMF_bits(address_bits);
         }
 
-        args.memflags |= MEMF_node(XENMEMF_get_node(reservation.mem_flags));
-        if ( reservation.mem_flags & XENMEMF_exact_node_request )
-            args.memflags |= MEMF_exact_node;
+        if ( unlikely(!propagate_node(reservation.mem_flags, &args.memflags)) )
+            return -EINVAL;
 
         if ( op == XENMEM_populate_physmap
              && (reservation.mem_flags & XENMEMF_populate_on_demand) )
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -610,9 +610,13 @@ static struct page_info *alloc_heap_page
         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 >= 0);
     ASSERT(zone_lo <= zone_hi);
     ASSERT(zone_hi < NR_ZONES);
 

[-- Attachment #5: xsa231-4.7.patch --]
[-- Type: application/octet-stream, Size: 3382 bytes --]

From: George Dunlap <george.dunlap@citrix.com>
Subject: 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 XSA-231.

Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -390,6 +390,31 @@ static void decrease_reservation(struct
     a->nr_done = i;
 }
 
+static bool_t 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 1;
+
+    if ( is_hardware_domain(currd) || is_control_domain(currd) )
+    {
+        if ( XENMEMF_get_node(xmf) >= MAX_NUMNODES )
+            return 0;
+
+        *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 0;
+
+    return 1;
+}
+
 static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 {
     struct xen_memory_exchange exch;
@@ -462,6 +487,12 @@ static long memory_exchange(XEN_GUEST_HA
         }
     }
 
+    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 )
     {
@@ -480,7 +511,6 @@ static long memory_exchange(XEN_GUEST_HA
         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);
@@ -834,12 +864,8 @@ static int construct_memop_from_reservat
         }
         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;
 }
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -711,9 +711,13 @@ static struct page_info *alloc_heap_page
         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);
     ASSERT(zone_lo <= zone_hi);
     ASSERT(zone_hi < NR_ZONES);
 

[-- Attachment #6: xsa231-4.9.patch --]
[-- Type: application/octet-stream, Size: 3394 bytes --]

From: George Dunlap <george.dunlap@citrix.com>
Subject: 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 XSA-231.

Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -411,6 +411,31 @@ static void decrease_reservation(struct
     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;
@@ -483,6 +508,12 @@ static long memory_exchange(XEN_GUEST_HA
         }
     }
 
+    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 )
     {
@@ -501,7 +532,6 @@ static long memory_exchange(XEN_GUEST_HA
         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);
@@ -864,12 +894,8 @@ static int construct_memop_from_reservat
         }
         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;
 }
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -706,9 +706,13 @@ static struct page_info *alloc_heap_page
         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);
     ASSERT(zone_lo <= zone_hi);
     ASSERT(zone_hi < NR_ZONES);
 

[-- Attachment #7: Type: text/plain, Size: 127 bytes --]

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

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-09-12 12:03 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-12 12:03 Xen Security Advisory 231 (CVE-2017-14316) - Missing NUMA node parameter verification Xen.org security team

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).