[Xen-changelog] [xen master] x86/svm: Don't clobber eax and edx if an RDMSR intercept fails

[patchbot@xxxxxxx]

commit a0b4e3c0681a11b765fe218fba0ba4ebb9fa56c5
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Nov 2 14:36:49 2016 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Thu Nov 10 15:34:42 2016 +0000

    x86/svm: Don't clobber eax and edx if an RDMSR intercept fails
    
    The original code has a bug; eax and edx get unconditionally updated even 
when
    hvm_msr_read_intercept() doesn't return X86EMUL_OKAY.
    
    It is only by blind luck (vmce_rdmsr() eagerly initialising its msr_content
    pointer) that this isn't an information leak into guests.
    
    While fixing this bug, reduce the scope of msr_content and initialise it to 
0.
    This makes it obvious that a stack leak won't occur, even if there were to 
be
    a buggy codepath in hvm_msr_read_intercept().
    
    Also make some non-functional improvements.  Make the insn_len calculation
    common, and reduce the quantity of explicit casting by making better use of
    the existing register names.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
    Release-acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 xen/arch/x86/hvm/svm/svm.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 16427f6..c9dbbea 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1948,26 +1948,28 @@ static int svm_msr_write_intercept(unsigned int msr, 
uint64_t msr_content)
 
 static void svm_do_msr_access(struct cpu_user_regs *regs)
 {
-    int rc, inst_len;
-    struct vcpu *v = current;
-    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
-    uint64_t msr_content;
+    struct vcpu *curr = current;
+    bool rdmsr = curr->arch.hvm_svm.vmcb->exitinfo1 == 0;
+    int rc, inst_len = __get_instruction_length(
+        curr, rdmsr ? INSTR_RDMSR : INSTR_WRMSR);
+
+    if ( inst_len == 0 )
+        return;
 
-    if ( vmcb->exitinfo1 == 0 )
+    if ( rdmsr )
     {
-        if ( (inst_len = __get_instruction_length(v, INSTR_RDMSR)) == 0 )
-            return;
-        rc = hvm_msr_read_intercept(regs->ecx, &msr_content);
-        regs->eax = (uint32_t)msr_content;
-        regs->edx = (uint32_t)(msr_content >> 32);
+        uint64_t msr_content = 0;
+
+        rc = hvm_msr_read_intercept(regs->_ecx, &msr_content);
+        if ( rc == X86EMUL_OKAY )
+        {
+            regs->rax = (uint32_t)msr_content;
+            regs->rdx = (uint32_t)(msr_content >> 32);
+        }
     }
     else
-    {
-        if ( (inst_len = __get_instruction_length(v, INSTR_WRMSR)) == 0 )
-            return;
-        msr_content = ((uint64_t)regs->edx << 32) | (uint32_t)regs->eax;
-        rc = hvm_msr_write_intercept(regs->ecx, msr_content, 1);
-    }
+        rc = hvm_msr_write_intercept(regs->_ecx,
+                                     (regs->rdx << 32) | regs->_eax, 1);
 
     if ( rc == X86EMUL_OKAY )
         __update_guest_eip(regs, inst_len);
--
generated by git-patchbot for /home/xen/git/xen.git#master

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