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

[patchbot@xxxxxxx]

commit 8861999ff548551893cc660d85dd037440882972
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Sep 28 17:01:09 2016 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Sep 28 17:01:09 2016 +0200

    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>
    master commit: d4a322557ae98cccdf90a0f442a29e1f5d76378a
    master date: 2016-09-13 10:43:59 +0100
---
 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 db5681a..5aa0ed9 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -869,19 +869,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,
@@ -902,6 +908,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
         {
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.6

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