[Xen-changelog] [xen master] x86/xstate: Fix latent bugs in compress_xsave_states()

[patchbot@xxxxxxx]

commit 773522000cc17f6f4323a4d97423790138ea98f2
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:44:09 2016 +0100

    x86/xstate: Fix latent bugs in compress_xsave_states()
    
    compress_xsave_states() mustn't read xstate_bv or xcomp_bv before first
    confirming that the input buffer is large enough.  It also doesn't cope with
    compressed input.  Make all of these problems the callers responsbility to
    ensure.
    
    Simplify the decompression logic by inlining get_xsave_addr().  As xstate_bv
    is previously validated, dest won't ever been NULL.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/xstate.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 6739c83..85a0116 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -154,15 +154,6 @@ static void setup_xstate_comp(uint16_t *comp_offsets,
     ASSERT(offset <= xsave_cntxt_size);
 }
 
-static void *get_xsave_addr(struct xsave_struct *xsave,
-                            const uint16_t *comp_offsets,
-                            unsigned int xfeature_idx)
-{
-    ASSERT(xsave_area_compressed(xsave));
-    return (1ul << xfeature_idx) & xsave->xsave_hdr.xstate_bv ?
-           (void *)xsave + comp_offsets[xfeature_idx] : NULL;
-}
-
 /*
  * Serialise a vcpus xsave state into a representation suitable for the
  * toolstack.
@@ -229,15 +220,29 @@ void expand_xsave_states(struct vcpu *v, void *dest, 
unsigned int size)
     }
 }
 
+/*
+ * Deserialise a toolstack's xsave state representation suitably for a vcpu.
+ *
+ * Internally a vcpus xsave state may be compressed or uncompressed, depending
+ * on the features in use, but the ABI with the toolstack is strictly
+ * uncompressed.
+ *
+ * It is the callers responsibility to ensure that the source buffer contains
+ * xsave state, is uncompressed, and is exactly the right size.
+ */
 void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
 {
     struct xsave_struct *xsave = v->arch.xsave_area;
+    void *dest;
     uint16_t comp_offsets[sizeof(xfeature_mask)*8];
-    u64 xstate_bv = ((const struct xsave_struct *)src)->xsave_hdr.xstate_bv;
-    u64 valid;
+    u64 xstate_bv, valid;
 
+    BUG_ON(!v->arch.xcr0_accum);
+    BUG_ON(size != xstate_ctxt_size(v->arch.xcr0_accum));
     ASSERT(!xsave_area_compressed(src));
 
+    xstate_bv = ((const struct xsave_struct *)src)->xsave_hdr.xstate_bv;
+
     if ( !(v->arch.xcr0_accum & XSTATE_XSAVES_ONLY) )
     {
         memcpy(xsave, src, size);
@@ -260,18 +265,22 @@ void compress_xsave_states(struct vcpu *v, const void 
*src, unsigned int size)
      * Copy each region from the non-compacted offset to the
      * possibly compacted offset.
      */
+    dest = xsave;
     valid = xstate_bv & ~XSTATE_FP_SSE;
     while ( valid )
     {
         u64 feature = valid & -valid;
         unsigned int index = fls(feature) - 1;
-        void *dest = get_xsave_addr(xsave, comp_offsets, index);
 
-        if ( dest )
-        {
-            ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= size);
-            memcpy(dest, src + xstate_offsets[index], xstate_sizes[index]);
-        }
+        /*
+         * We previously verified xstate_bv.  If we don't have valid
+         * comp_offset[] information, something is very broken.
+         */
+        BUG_ON(!comp_offsets[index]);
+        BUG_ON((xstate_offsets[index] + xstate_sizes[index]) > size);
+
+        memcpy(dest + comp_offsets[index], src + xstate_offsets[index],
+               xstate_sizes[index]);
 
         valid &= ~feature;
     }
--
generated by git-patchbot for /home/xen/git/xen.git#master

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