[Xen-changelog] [xen master] x86/domctl: Fix TOCTOU race with the use of XEN_DOMCTL_getvcpuextstate

[patchbot@xxxxxxx]

commit d4a322557ae98cccdf90a0f442a29e1f5d76378a
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Mon Sep 12 10:30:00 2016 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Sep 13 10:43:59 2016 +0100

    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@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/domctl.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index c9355ce..815bd33 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1054,19 +1054,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,
@@ -1103,6 +1109,10 @@ long arch_do_domctl(
            }
 
             vcpu_unpause(v);
+
+            /* Specify how much data we actually wrote into the buffer. */
+            if ( !ret )
+                evc->size = size;
         }
         else
         {
--
generated by git-patchbot for /home/xen/git/xen.git#master

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