File 57d7ca5f-x86-domctl-fix-TOCTOU-race-in-XEN_DOMC... of Package xen.3680

Overview Repositories Revisions Requests Users Attributes Meta

File 57d7ca5f-x86-domctl-fix-TOCTOU-race-in-XEN_DOMCTL_getvcpuextstate.patch of Package xen.3680

# Commit d4a322557ae98cccdf90a0f442a29e1f5d76378a
# Date 2016-09-13 10:43:59 +0100
# Author Andrew Cooper <andrew.cooper3@citrix.com>
# Committer Andrew Cooper <andrew.cooper3@citrix.com>
x86/domctl: Fix TOCTOU race with the use of XEN_DOMCTL_getvcpuextstate

A toolstack must call XEN_DOMCTL_getvcpuextstate twice; first to find the size
of the buffer to use, and a second time to get the actual content.

The reported size was based on v->arch.xcr0_accum, but a guest which extends
its xcr0_accum between the two hypercalls will cause the toolstack to fail the
evc->size != size check, as the provided buffer is now too small.  This causes
a hard error during the final phase of migration.

Instead, return a size based on xfeature_mask, which is the maximum size Xen
will ever permit.  The hypercall must now tolerate a toolstack-provided buffer
which is overly large (for the case where a guest isn't using all available
xsave states), and should write back how much data was actually written into
the buffer.

As the query for size now has no dependence on vcpu state, the vcpu_pause()
can be omitted for a small performance improvement.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1113,19 +1113,25 @@ long arch_do_domctl(
             unsigned int size;
 
             ret = 0;
-            vcpu_pause(v);
 
-            size = PV_XSAVE_SIZE(v->arch.xcr0_accum);
             if ( (!evc->size && !evc->xfeature_mask) ||
                  guest_handle_is_null(evc->buffer) )
             {
+                /*
+                 * A query for the size of buffer to use.  Must return the
+                 * maximum size we ever might hand back to userspace, bearing
+                 * in mind that the vcpu might increase its xcr0_accum between
+                 * this query for size, and the following query for data.
+                 */
                 evc->xfeature_mask = xfeature_mask;
-                evc->size = size;
-                vcpu_unpause(v);
+                evc->size = PV_XSAVE_SIZE(xfeature_mask);
                 goto vcpuextstate_out;
             }
 
-            if ( evc->size != size || evc->xfeature_mask != xfeature_mask )
+            vcpu_pause(v);
+            size = PV_XSAVE_SIZE(v->arch.xcr0_accum);
+
+            if ( evc->size < size || evc->xfeature_mask != xfeature_mask )
                 ret = -EINVAL;
 
             if ( !ret && copy_to_guest_offset(evc->buffer, offset,
@@ -1146,6 +1152,10 @@ long arch_do_domctl(
                 ret = -EFAULT;
 
             vcpu_unpause(v);
+
+            /* Specify how much data we actually wrote into the buffer. */
+            if ( !ret )
+                evc->size = size;
         }
         else
         {

Places

File 57d7ca5f-x86-domctl-fix-TOCTOU-race-in-XEN_DOMCTL_getvcpuextstate.patch of Package xen.3680

Places