memory: properly check guest memory ranges in XENMEM_exchange handling

The use of guest_handle_okay() here (as introduced by the XSA-29 fix)
is insufficient here, guest_handle_subrange_okay() needs to be used
instead.

Note that the uses are okay in
- XENMEM_add_to_physmap_batch handling due to the size field being only
  16 bits wide,
- livepatch_list() due to the limit of 1024 enforced on the
  number-of-entries input (leaving aside the fact that this can be
  called by a privileged domain only anyway),
- compat mode handling due to counts there being limited to 32 bits,
- everywhere else due to guest arrays being accessed sequentially from
  index zero.

This is CVE-2017-7228 / XSA-212.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 938fd2586eb081bcbd694f4c1f09ae6a263b0d90
master date: 2017-04-04 14:47:46 +0200

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 55db10b3a5a5a3b675ac836f366453b398a3dc72..fbaaea829fec6b815fac422f74051537878c4f7f 100644 (file)
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -402,8 +402,8 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
         goto fail_early;
     }
 
-    if ( !guest_handle_okay(exch.in.extent_start, exch.in.nr_extents) ||
-         !guest_handle_okay(exch.out.extent_start, exch.out.nr_extents) )
+    if ( !guest_handle_subrange_okay(exch.in.extent_start, exch.nr_exchanged,
+                                     exch.in.nr_extents - 1) )
     {
         rc = -EFAULT;
         goto fail_early;
@@ -413,11 +413,27 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
     {
         in_chunk_order  = exch.out.extent_order - exch.in.extent_order;
         out_chunk_order = 0;
+
+        if ( !guest_handle_subrange_okay(exch.out.extent_start,
+                                         exch.nr_exchanged >> in_chunk_order,
+                                         exch.out.nr_extents - 1) )
+        {
+            rc = -EFAULT;
+            goto fail_early;
+        }
     }
     else
     {
         in_chunk_order  = 0;
         out_chunk_order = exch.in.extent_order - exch.out.extent_order;
+
+        if ( !guest_handle_subrange_okay(exch.out.extent_start,
+                                         exch.nr_exchanged << out_chunk_order,
+                                         exch.out.nr_extents - 1) )
+        {
+            rc = -EFAULT;
+            goto fail_early;
+        }
     }
 
     d = rcu_lock_domain_by_any_id(exch.in.domid);

diff --git a/xen/include/asm-x86/x86_64/uaccess.h b/xen/include/asm-x86/x86_64/uaccess.h
index 953abe769383ff98c0f3474567bcef83da1f3113..4275e66f401f587848268023bd5732f77232de4a 100644 (file)
--- a/xen/include/asm-x86/x86_64/uaccess.h
+++ b/xen/include/asm-x86/x86_64/uaccess.h
@@ -29,8 +29,9 @@ extern void *xlat_malloc(unsigned long *xlat_page_current, size_t size);
 /*
  * Valid if in +ve half of 48-bit address space, or above Xen-reserved area.
  * This is also valid for range checks (addr, addr+size). As long as the
- * start address is outside the Xen-reserved area then we will access a
- * non-canonical address (and thus fault) before ever reaching VIRT_START.
+ * start address is outside the Xen-reserved area, sequential accesses
+ * (starting at addr) will hit a non-canonical address (and thus fault)
+ * before ever reaching VIRT_START.
  */
 #define __addr_ok(addr) \
     (((unsigned long)(addr) < (1UL<<47)) || \
@@ -40,7 +41,8 @@ extern void *xlat_malloc(unsigned long *xlat_page_current, size_t size);
     (__addr_ok(addr) || is_compat_arg_xlat_range(addr, size))
 
 #define array_access_ok(addr, count, size) \
-    (access_ok(addr, (count)*(size)))
+    (likely(((count) ?: 0UL) < (~0UL / (size))) && \
+     access_ok(addr, (count) * (size)))
 
 #define __compat_addr_ok(d, addr) \
     ((unsigned long)(addr) < HYPERVISOR_COMPAT_VIRT_START(d))