[Xen-devel] [PATCH 2/6] x86/domctl: Fix TOCTOU race with the use of XEN_DOMCTL_getvcpuextstate

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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 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>
---
CC: 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
         {
-- 
2.1.4


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