[4/4] x86/xstate: extend validation to cover full header

Commit Message

Jan Beulich Jan. 29, 2016, 10:30 a.m. UTC

Since we never hand out compacted state, at least for now we're also
not going to accept such.

Reported-by: Harmandeep Kaur <[e-mail]>
Signed-off-by: Jan Beulich <[e-mail]>

Comments

Andrew Cooper Jan. 29, 2016, 4:55 p.m. UTC | #1

On 29/01/16 10:30, Jan Beulich wrote:
> Since we never hand out compacted state, at least for now we're also
> not going to accept such.
>
> Reported-by: Harmandeep Kaur <[e-mail]>
> Signed-off-by: Jan Beulich <[e-mail]>

Reviewed-by: Andrew Cooper <[e-mail]>

Patch

--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -964,7 +964,7 @@  long arch_do_domctl(
             {
                 if ( evc->size >= 2 * sizeof(uint64_t) + XSTATE_AREA_MIN_SIZE )
                     ret = validate_xstate(_xcr0, _xcr0_accum,
-                                          _xsave_area->xsave_hdr.xstate_bv);
+                                          &_xsave_area->xsave_hdr);
             }
             else if ( !_xcr0 )
                 ret = 0;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2188,6 +2188,19 @@  static int hvm_save_cpu_xsave_states(str
     return 0;
 }
 
+/*
+ * Structure layout conformity checks, documenting correctness of the cast in
+ * the invocation of validate_xstate() below.
+ * Leverage CONFIG_COMPAT machinery to perform this.
+ */
+#define xen_xsave_hdr xsave_hdr
+#define compat_xsave_hdr hvm_hw_cpu_xsave_hdr
+CHECK_FIELD_(struct, xsave_hdr, xstate_bv);
+CHECK_FIELD_(struct, xsave_hdr, xcomp_bv);
+CHECK_FIELD_(struct, xsave_hdr, reserved);
+#undef compat_xsave_hdr
+#undef xen_xsave_hdr
+
 static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
 {
     unsigned int vcpuid, size;
@@ -2243,7 +2256,7 @@  static int hvm_load_cpu_xsave_states(str
     h->cur += desc->length;
 
     err = validate_xstate(ctxt->xcr0, ctxt->xcr0_accum,
-                          ctxt->save_area.xsave_hdr.xstate_bv);
+                          (const void *)&ctxt->save_area.xsave_hdr);
     if ( err )
     {
         printk(XENLOG_G_WARNING
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -614,17 +614,24 @@  static bool_t valid_xcr0(u64 xcr0)
     return !(xcr0 & XSTATE_BNDREGS) == !(xcr0 & XSTATE_BNDCSR);
 }
 
-int validate_xstate(u64 xcr0, u64 xcr0_accum, u64 xstate_bv)
+int validate_xstate(u64 xcr0, u64 xcr0_accum, const struct xsave_hdr *hdr)
 {
-    if ( (xstate_bv & ~xcr0_accum) ||
+    unsigned int i;
+
+    if ( (hdr->xstate_bv & ~xcr0_accum) ||
          (xcr0 & ~xcr0_accum) ||
          !valid_xcr0(xcr0) ||
          !valid_xcr0(xcr0_accum) )
         return -EINVAL;
 
-    if ( xcr0_accum & ~xfeature_mask )
+    if ( (xcr0_accum & ~xfeature_mask) ||
+         hdr->xcomp_bv )
         return -EOPNOTSUPP;
 
+    for ( i = 0; i < ARRAY_SIZE(hdr->reserved); ++i )
+        if ( hdr->reserved[i] )
+            return -EIO;
+
     return 0;
 }
 
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -72,14 +72,13 @@  struct __attribute__((aligned (64))) xsa
         };
     } fpu_sse;
 
-    struct {
+    struct xsave_hdr {
         u64 xstate_bv;
         u64 xcomp_bv;
         u64 reserved[6];
     } xsave_hdr;                             /* The 64-byte header */
 
-    struct { char x[XSTATE_YMM_SIZE]; } ymm; /* YMM */
-    char   data[];                           /* Future new states */
+    char data[];                             /* Variable layout states */
 };
 
 /* extended state operations */
@@ -90,7 +89,8 @@  uint64_t get_msr_xss(void);
 void xsave(struct vcpu *v, uint64_t mask);
 void xrstor(struct vcpu *v, uint64_t mask);
 bool_t xsave_enabled(const struct vcpu *v);
-int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum, u64 xstate_bv);
+int __must_check validate_xstate(u64 xcr0, u64 xcr0_accum,
+                                 const struct xsave_hdr *);
 int __must_check handle_xsetbv(u32 index, u64 new_bv);
 void expand_xsave_states(struct vcpu *v, void *dest, unsigned int size);
 void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size);
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -564,12 +564,11 @@  struct hvm_hw_cpu_xsave {
     struct {
         struct { char x[512]; } fpu_sse;
 
-        struct {
+        struct hvm_hw_cpu_xsave_hdr {
             uint64_t xstate_bv;         /* Updated by XRSTOR */
-            uint64_t reserved[7];
+            uint64_t xcomp_bv;          /* Updated by XRSTOR{C,S} */
+            uint64_t reserved[6];
         } xsave_hdr;                    /* The 64-byte header */
-
-        struct { char x[0]; } ymm;    /* YMM */
     } save_area;
 };