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.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Boris Ostrovsky <boris.ostrov...@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
CC: Wei Liu <wei.l...@citrix.com>
---
xen/arch/x86/hvm/svm/svm.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 16427f6..6530e22 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;
+ bool rdmsr = v->arch.hvm_svm.vmcb->exitinfo1 == 0;
+ int rc, inst_len = __get_instruction_length(
+ v, 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);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
