[Xen-changelog] [xen stable-4.2] x86/domctl: two functional fixes to XEN_DOMCTL_[gs]etvcpuextstate

[patchbot@xxxxxxx]

commit 5bf9449364dfbc845ddb4b8f20ae9df56aaaa692
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Tue Jun 24 10:11:54 2014 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Jun 24 10:11:54 2014 +0200

    x86/domctl: two functional fixes to XEN_DOMCTL_[gs]etvcpuextstate
    
    Interacting with the vcpu itself should be protected by vcpu_pause().
    Buggy/naive toolstacks might encounter adverse interaction with a vcpu 
context
    switch, or increase of xcr0_accum.  There are no much problems with current
    in-tree code.
    
    Explicitly permit a NULL guest handle as being a request for size.  It is 
the
    prevailing Xen style, and without it, valgrind's ioctl handler is unable to
    determine whether evc->buffer actually got written to.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    
    x86/domctl: further fix to XEN_DOMCTL_[gs]etvcpuextstate
    
    Do not clobber errors from certain codepaths.  Clobbering of -EINVAL from
    failing "evc->size <= PV_XSAVE_SIZE(_xcr0_accum)" was a pre-existing bug.
    
    However, clobbering -EINVAL/-EFAULT from the get codepath was a bug
    unintentionally introduced by 090ca8c1 "x86/domctl: two functional fixes to
    XEN_DOMCTL_[gs]etvcpuextstate".
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    master commit: 090ca8c155b7321404ea7713a28aaedb7ac4fffd
    master date: 2014-06-05 17:52:57 +0200
    master commit: 895661ae98f0249f50280b4acfb9dda70b76d7e9
    master date: 2014-06-10 12:03:16 +0200
---
 xen/arch/x86/domctl.c |   55 +++++++++++++++++++++++++-----------------------
 1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 099fb4b..bb80b23 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1413,45 +1413,48 @@ long arch_do_domctl(
              ((v = d->vcpu[evc->vcpu]) == NULL) )
             goto vcpuextstate_out;
 
+        ret = -EINVAL;
+        if ( v == current ) /* no vcpu_pause() */
+            goto vcpuextstate_out;
+
         if ( domctl->cmd == XEN_DOMCTL_getvcpuextstate )
         {
-            unsigned int size = PV_XSAVE_SIZE(v->arch.xcr0_accum);
+            unsigned int size;
 
-            if ( !evc->size && !evc->xfeature_mask )
+            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) )
             {
                 evc->xfeature_mask = xfeature_mask;
                 evc->size = size;
-                ret = 0;
+                vcpu_unpause(v);
                 goto vcpuextstate_out;
             }
+
             if ( evc->size != size || evc->xfeature_mask != xfeature_mask )
-            {
                 ret = -EINVAL;
-                goto vcpuextstate_out;
-            }
-            if ( copy_to_guest_offset(domctl->u.vcpuextstate.buffer,
-                                      offset, (void *)&v->arch.xcr0,
-                                      sizeof(v->arch.xcr0)) )
-            {
+
+            if ( !ret && copy_to_guest_offset(evc->buffer, offset,
+                                              (void *)&v->arch.xcr0,
+                                              sizeof(v->arch.xcr0)) )
                 ret = -EFAULT;
-                goto vcpuextstate_out;
-            }
+
             offset += sizeof(v->arch.xcr0);
-            if ( copy_to_guest_offset(domctl->u.vcpuextstate.buffer,
-                                      offset, (void *)&v->arch.xcr0_accum,
-                                      sizeof(v->arch.xcr0_accum)) )
-            {
+            if ( !ret && copy_to_guest_offset(evc->buffer, offset,
+                                              (void *)&v->arch.xcr0_accum,
+                                              sizeof(v->arch.xcr0_accum)) )
                 ret = -EFAULT;
-                goto vcpuextstate_out;
-            }
+
             offset += sizeof(v->arch.xcr0_accum);
-            if ( copy_to_guest_offset(domctl->u.vcpuextstate.buffer,
-                                      offset, (void *)v->arch.xsave_area,
-                                      size - 2 * sizeof(uint64_t)) )
-            {
+            if ( !ret && copy_to_guest_offset(evc->buffer, offset,
+                                              (void *)v->arch.xsave_area,
+                                              size - 2 * sizeof(uint64_t)) )
                 ret = -EFAULT;
-                goto vcpuextstate_out;
-            }
+
+            vcpu_unpause(v);
         }
         else
         {
@@ -1500,12 +1503,14 @@ long arch_do_domctl(
 
             if ( evc->size <= PV_XSAVE_SIZE(_xcr0_accum) )
             {
+                vcpu_pause(v);
                 v->arch.xcr0 = _xcr0;
                 v->arch.xcr0_accum = _xcr0_accum;
                 if ( _xcr0_accum & XSTATE_NONLAZY )
                     v->arch.nonlazy_xstate_used = 1;
                 memcpy(v->arch.xsave_area, _xsave_area,
                        evc->size - 2 * sizeof(uint64_t));
+                vcpu_unpause(v);
             }
             else
                 ret = -EINVAL;
@@ -1513,8 +1518,6 @@ long arch_do_domctl(
             xfree(receive_buf);
         }
 
-        ret = 0;
-
     vcpuextstate_out:
         rcu_unlock_domain(d);
         if ( (domctl->cmd == XEN_DOMCTL_getvcpuextstate) &&
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.2

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog