x86: segment attribute handling adjustments

Null selector loads into SS (possible in 64-bit mode only, and only in
rings other than ring 3) must not alter SS.DPL. (This was found to be
an issue on KVM, and fixed in Linux commit 33ab91103b.)

Further arch_set_info_hvm_guest() didn't make sure that the ASSERT()s
in hvm_set_segment_register() wouldn't trigger: Add further checks, but
tolerate (adjust) clear accessed (CS, SS, DS, ES) and busy (TR) bits.

Finally the setting of the accessed bits for user segments was lost by
commit dd5c85e312 ("x86/hvm: Reposition the modification of raw segment
data from the VMCB/VMCS"), yet VMX requires them to be set for usable
segments. Add respective ASSERT()s (the only path not properly setting
them was arch_set_info_hvm_guest()).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 340905475666e43300ba85ab1c33d577485c2922..5a4e7f9166104964ed9299e8672696fb7be1fcd5 100644 (file)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1294,16 +1294,24 @@ static inline int check_segment(struct segment_register *reg,
         return 0;
     }
 
-    if ( seg != x86_seg_tr && !reg->attr.fields.s )
+    if ( seg == x86_seg_tr )
     {
-        gprintk(XENLOG_ERR,
-                "System segment provided for a code or data segment\n");
-        return -EINVAL;
-    }
+        if ( reg->attr.fields.s )
+        {
+            gprintk(XENLOG_ERR, "Code or data segment provided for TR\n");
+            return -EINVAL;
+        }
 
-    if ( seg == x86_seg_tr && reg->attr.fields.s )
+        if ( reg->attr.fields.type != SYS_DESC_tss_busy )
+        {
+            gprintk(XENLOG_ERR, "Non-32-bit-TSS segment provided for TR\n");
+            return -EINVAL;
+        }
+    }
+    else if ( !reg->attr.fields.s )
     {
-        gprintk(XENLOG_ERR, "Code or data segment provided for TR\n");
+        gprintk(XENLOG_ERR,
+                "System segment provided for a code or data segment\n");
         return -EINVAL;
     }
 
@@ -1366,7 +1374,8 @@ int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
 #define SEG(s, r) ({                                                        \
     s = (struct segment_register){ .base = (r)->s ## _base,                 \
                                    .limit = (r)->s ## _limit,               \
-                                   .attr.bytes = (r)->s ## _ar };           \
+                                   .attr.bytes = (r)->s ## _ar |            \
+                                       (x86_seg_##s != x86_seg_tr ? 1 : 2) }; \
     check_segment(&s, x86_seg_ ## s); })
 
         rc = SEG(cs, regs);

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 63748dc049dd6024184a312172d7913ed96133a6..6ab60d20e30b40e7bc40aade9e9d2214d1e9993e 100644 (file)
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5731,6 +5731,7 @@ void hvm_set_segment_register(struct vcpu *v, enum x86_segment seg,
     case x86_seg_cs:
         ASSERT(reg->attr.fields.p);                  /* Usable. */
         ASSERT(reg->attr.fields.s);                  /* User segment. */
+        ASSERT(reg->attr.fields.type & 0x1);         /* Accessed. */
         ASSERT((reg->base >> 32) == 0);              /* Upper bits clear. */
         break;
 
@@ -5740,6 +5741,7 @@ void hvm_set_segment_register(struct vcpu *v, enum x86_segment seg,
             ASSERT(reg->attr.fields.s);              /* User segment. */
             ASSERT(!(reg->attr.fields.type & 0x8));  /* Data segment. */
             ASSERT(reg->attr.fields.type & 0x2);     /* Writeable. */
+            ASSERT(reg->attr.fields.type & 0x1);     /* Accessed. */
             ASSERT((reg->base >> 32) == 0);          /* Upper bits clear. */
         }
         break;
@@ -5755,6 +5757,8 @@ void hvm_set_segment_register(struct vcpu *v, enum x86_segment seg,
             if ( reg->attr.fields.type & 0x8 )
                 ASSERT(reg->attr.fields.type & 0x2); /* Readable. */
 
+            ASSERT(reg->attr.fields.type & 0x1);     /* Accessed. */
+
             if ( seg == x86_seg_fs || seg == x86_seg_gs )
                 ASSERT(is_canonical_address(reg->base));
             else

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 1c76379bb69909a993bf2b7afc31f286e249f4ab..2810c73ce67b527d3c1f2cd1ddb9ae20717b3cca 100644 (file)
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1471,6 +1471,11 @@ protmode_load_seg(
         else
             sreg->attr.bytes = 0;
         sreg->sel = sel;
+
+        /* Since CPL == SS.DPL, we need to put back DPL. */
+        if ( seg == x86_seg_ss )
+            sreg->attr.fields.dpl = sel;
+
         return X86EMUL_OKAY;
     }