[Xen-changelog] [xen master] x86/PV: writes of %fs and %gs base MSRs require canonical addresses

[patchbot@xxxxxxx]

commit f3fa3abf3e61fb1f25ce721e14ac324dda67311f
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Nov 22 13:46:28 2016 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Nov 22 13:46:28 2016 +0100

    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@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/arch/x86/domain.c | 8 +++++++-
 xen/arch/x86/traps.c  | 6 +++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 1bd5eb6..eae643f 100644
--- 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 d56d76e..b464211 100644
--- 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;
--
generated by git-patchbot for /home/xen/git/xen.git#master

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