x86/PV: writes of %fs and %gs base MSRs require canonical addresses

Commit c42494acb2 ("x86: fix FS/GS base handling when using the
fsgsbase feature") replaced the use of wrmsr_safe() on these paths
without recognizing that wr{f,g}sbase() use just wrmsrl() and that the
WR{F,G}SBASE instructions also raise #GP for non-canonical input.

Similarly arch_set_info_guest() needs to prevent non-canonical
addresses from getting stored into state later to be loaded by context
switch code. For consistency also check stack pointers and LDT base.
DR0..3, otoh, already get properly checked in set_debugreg() (albeit
we discard the error there).

The SHADOW_GS_BASE check isn't strictly necessary, but I think we
better avoid trying the WRMSR if we know it's going to fail.

This is CVE-2016-9385 / XSA-193.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
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 1bd5eb607cacc59495a0da5ad2470c0a8d255efb..eae643ff225d22d082ef3d6f90bb313b473fc271 100644 (file)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -897,7 +897,13 @@ int arch_set_info_guest(
     {
         if ( !compat )
         {
-            if ( !is_canonical_address(c.nat->user_regs.eip) ||
+            if ( !is_canonical_address(c.nat->user_regs.rip) ||
+                 !is_canonical_address(c.nat->user_regs.rsp) ||
+                 !is_canonical_address(c.nat->kernel_sp) ||
+                 (c.nat->ldt_ents && !is_canonical_address(c.nat->ldt_base)) ||
+                 !is_canonical_address(c.nat->fs_base) ||
+                 !is_canonical_address(c.nat->gs_base_kernel) ||
+                 !is_canonical_address(c.nat->gs_base_user) ||
                  !is_canonical_address(c.nat->event_callback_eip) ||
                  !is_canonical_address(c.nat->syscall_callback_eip) ||
                  !is_canonical_address(c.nat->failsafe_callback_eip) )

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d56d76ead887f3ece96f505dfafe584833991d71..b4642118287363d89c051d02a78c264a4605314a 100644 (file)
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2565,21 +2565,21 @@ static int priv_op_write_msr(unsigned int reg, uint64_t val,
         int rc;
 
     case MSR_FS_BASE:
-        if ( is_pv_32bit_domain(currd) )
+        if ( is_pv_32bit_domain(currd) || !is_canonical_address(val) )
             break;
         wrfsbase(val);
         curr->arch.pv_vcpu.fs_base = val;
         return X86EMUL_OKAY;
 
     case MSR_GS_BASE:
-        if ( is_pv_32bit_domain(currd) )
+        if ( is_pv_32bit_domain(currd) || !is_canonical_address(val) )
             break;
         wrgsbase(val);
         curr->arch.pv_vcpu.gs_base_kernel = val;
         return X86EMUL_OKAY;
 
     case MSR_SHADOW_GS_BASE:
-        if ( is_pv_32bit_domain(currd) ||
+        if ( is_pv_32bit_domain(currd) || !is_canonical_address(val) ||
              wrmsr_safe(MSR_SHADOW_GS_BASE, val) )
             break;
         curr->arch.pv_vcpu.gs_base_user = val;