b136801
From: Nadav Amit <namit@cs.technion.ac.il>
b136801
Date: Fri, 24 Oct 2014 17:07:16 +0200
b136801
Subject: [PATCH] KVM: x86: Emulator fixes for eip canonical checks on near
b136801
 branches
b136801
b136801
Before changing rip (during jmp, call, ret, etc.) the target should be asserted
b136801
to be canonical one, as real CPUs do.  During sysret, both target rsp and rip
b136801
should be canonical. If any of these values is noncanonical, a #GP exception
b136801
should occur.  The exception to this rule are syscall and sysenter instructions
b136801
in which the assigned rip is checked during the assignment to the relevant
b136801
MSRs.
b136801
b136801
This patch fixes the emulator to behave as real CPUs do for near branches.
b136801
Far branches are handled by the next patch.
b136801
b136801
This fixes CVE-2014-3647.
b136801
b136801
Cc: stable@vger.kernel.org
b136801
Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
b136801
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
b136801
---
b136801
 arch/x86/kvm/emulate.c | 78 ++++++++++++++++++++++++++++++++++----------------
b136801
 1 file changed, 54 insertions(+), 24 deletions(-)
b136801
b136801
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
b136801
index a85f438b6a47..e52e74feedb8 100644
b136801
--- a/arch/x86/kvm/emulate.c
b136801
+++ b/arch/x86/kvm/emulate.c
b136801
@@ -563,7 +563,8 @@ static int emulate_nm(struct x86_emulate_ctxt *ctxt)
b136801
 	return emulate_exception(ctxt, NM_VECTOR, 0, false);
b136801
 }
b136801
 
b136801
-static inline void assign_eip_near(struct x86_emulate_ctxt *ctxt, ulong dst)
b136801
+static inline int assign_eip_far(struct x86_emulate_ctxt *ctxt, ulong dst,
b136801
+			       int cs_l)
b136801
 {
b136801
 	switch (ctxt->op_bytes) {
b136801
 	case 2:
b136801
@@ -573,16 +574,25 @@ static inline void assign_eip_near(struct x86_emulate_ctxt *ctxt, ulong dst)
b136801
 		ctxt->_eip = (u32)dst;
b136801
 		break;
b136801
 	case 8:
b136801
+		if ((cs_l && is_noncanonical_address(dst)) ||
b136801
+		    (!cs_l && (dst & ~(u32)-1)))
b136801
+			return emulate_gp(ctxt, 0);
b136801
 		ctxt->_eip = dst;
b136801
 		break;
b136801
 	default:
b136801
 		WARN(1, "unsupported eip assignment size\n");
b136801
 	}
b136801
+	return X86EMUL_CONTINUE;
b136801
+}
b136801
+
b136801
+static inline int assign_eip_near(struct x86_emulate_ctxt *ctxt, ulong dst)
b136801
+{
b136801
+	return assign_eip_far(ctxt, dst, ctxt->mode == X86EMUL_MODE_PROT64);
b136801
 }
b136801
 
b136801
-static inline void jmp_rel(struct x86_emulate_ctxt *ctxt, int rel)
b136801
+static inline int jmp_rel(struct x86_emulate_ctxt *ctxt, int rel)
b136801
 {
b136801
-	assign_eip_near(ctxt, ctxt->_eip + rel);
b136801
+	return assign_eip_near(ctxt, ctxt->_eip + rel);
b136801
 }
b136801
 
b136801
 static u16 get_segment_selector(struct x86_emulate_ctxt *ctxt, unsigned seg)
b136801
@@ -1989,13 +1999,15 @@ static int em_grp45(struct x86_emulate_ctxt *ctxt)
b136801
 	case 2: /* call near abs */ {
b136801
 		long int old_eip;
b136801
 		old_eip = ctxt->_eip;
b136801
-		ctxt->_eip = ctxt->src.val;
b136801
+		rc = assign_eip_near(ctxt, ctxt->src.val);
b136801
+		if (rc != X86EMUL_CONTINUE)
b136801
+			break;
b136801
 		ctxt->src.val = old_eip;
b136801
 		rc = em_push(ctxt);
b136801
 		break;
b136801
 	}
b136801
 	case 4: /* jmp abs */
b136801
-		ctxt->_eip = ctxt->src.val;
b136801
+		rc = assign_eip_near(ctxt, ctxt->src.val);
b136801
 		break;
b136801
 	case 5: /* jmp far */
b136801
 		rc = em_jmp_far(ctxt);
b136801
@@ -2030,10 +2042,14 @@ static int em_cmpxchg8b(struct x86_emulate_ctxt *ctxt)
b136801
 
b136801
 static int em_ret(struct x86_emulate_ctxt *ctxt)
b136801
 {
b136801
-	ctxt->dst.type = OP_REG;
b136801
-	ctxt->dst.addr.reg = &ctxt->_eip;
b136801
-	ctxt->dst.bytes = ctxt->op_bytes;
b136801
-	return em_pop(ctxt);
b136801
+	int rc;
b136801
+	unsigned long eip;
b136801
+
b136801
+	rc = emulate_pop(ctxt, &eip, ctxt->op_bytes);
b136801
+	if (rc != X86EMUL_CONTINUE)
b136801
+		return rc;
b136801
+
b136801
+	return assign_eip_near(ctxt, eip);
b136801
 }
b136801
 
b136801
 static int em_ret_far(struct x86_emulate_ctxt *ctxt)
b136801
@@ -2314,7 +2330,7 @@ static int em_sysexit(struct x86_emulate_ctxt *ctxt)
b136801
 {
b136801
 	const struct x86_emulate_ops *ops = ctxt->ops;
b136801
 	struct desc_struct cs, ss;
b136801
-	u64 msr_data;
b136801
+	u64 msr_data, rcx, rdx;
b136801
 	int usermode;
b136801
 	u16 cs_sel = 0, ss_sel = 0;
b136801
 
b136801
@@ -2330,6 +2346,9 @@ static int em_sysexit(struct x86_emulate_ctxt *ctxt)
b136801
 	else
b136801
 		usermode = X86EMUL_MODE_PROT32;
b136801
 
b136801
+	rcx = reg_read(ctxt, VCPU_REGS_RCX);
b136801
+	rdx = reg_read(ctxt, VCPU_REGS_RDX);
b136801
+
b136801
 	cs.dpl = 3;
b136801
 	ss.dpl = 3;
b136801
 	ops->get_msr(ctxt, MSR_IA32_SYSENTER_CS, &msr_data);
b136801
@@ -2347,6 +2366,9 @@ static int em_sysexit(struct x86_emulate_ctxt *ctxt)
b136801
 		ss_sel = cs_sel + 8;
b136801
 		cs.d = 0;
b136801
 		cs.l = 1;
b136801
+		if (is_noncanonical_address(rcx) ||
b136801
+		    is_noncanonical_address(rdx))
b136801
+			return emulate_gp(ctxt, 0);
b136801
 		break;
b136801
 	}
b136801
 	cs_sel |= SELECTOR_RPL_MASK;
b136801
@@ -2355,8 +2377,8 @@ static int em_sysexit(struct x86_emulate_ctxt *ctxt)
b136801
 	ops->set_segment(ctxt, cs_sel, &cs, 0, VCPU_SREG_CS);
b136801
 	ops->set_segment(ctxt, ss_sel, &ss, 0, VCPU_SREG_SS);
b136801
 
b136801
-	ctxt->_eip = reg_read(ctxt, VCPU_REGS_RDX);
b136801
-	*reg_write(ctxt, VCPU_REGS_RSP) = reg_read(ctxt, VCPU_REGS_RCX);
b136801
+	ctxt->_eip = rdx;
b136801
+	*reg_write(ctxt, VCPU_REGS_RSP) = rcx;
b136801
 
b136801
 	return X86EMUL_CONTINUE;
b136801
 }
b136801
@@ -2897,10 +2919,13 @@ static int em_aad(struct x86_emulate_ctxt *ctxt)
b136801
 
b136801
 static int em_call(struct x86_emulate_ctxt *ctxt)
b136801
 {
b136801
+	int rc;
b136801
 	long rel = ctxt->src.val;
b136801
 
b136801
 	ctxt->src.val = (unsigned long)ctxt->_eip;
b136801
-	jmp_rel(ctxt, rel);
b136801
+	rc = jmp_rel(ctxt, rel);
b136801
+	if (rc != X86EMUL_CONTINUE)
b136801
+		return rc;
b136801
 	return em_push(ctxt);
b136801
 }
b136801
 
b136801
@@ -2932,11 +2957,12 @@ static int em_call_far(struct x86_emulate_ctxt *ctxt)
b136801
 static int em_ret_near_imm(struct x86_emulate_ctxt *ctxt)
b136801
 {
b136801
 	int rc;
b136801
+	unsigned long eip;
b136801
 
b136801
-	ctxt->dst.type = OP_REG;
b136801
-	ctxt->dst.addr.reg = &ctxt->_eip;
b136801
-	ctxt->dst.bytes = ctxt->op_bytes;
b136801
-	rc = emulate_pop(ctxt, &ctxt->dst.val, ctxt->op_bytes);
b136801
+	rc = emulate_pop(ctxt, &eip, ctxt->op_bytes);
b136801
+	if (rc != X86EMUL_CONTINUE)
b136801
+		return rc;
b136801
+	rc = assign_eip_near(ctxt, eip);
b136801
 	if (rc != X86EMUL_CONTINUE)
b136801
 		return rc;
b136801
 	rsp_increment(ctxt, ctxt->src.val);
b136801
@@ -3267,20 +3293,24 @@ static int em_lmsw(struct x86_emulate_ctxt *ctxt)
b136801
 
b136801
 static int em_loop(struct x86_emulate_ctxt *ctxt)
b136801
 {
b136801
+	int rc = X86EMUL_CONTINUE;
b136801
+
b136801
 	register_address_increment(ctxt, reg_rmw(ctxt, VCPU_REGS_RCX), -1);
b136801
 	if ((address_mask(ctxt, reg_read(ctxt, VCPU_REGS_RCX)) != 0) &&
b136801
 	    (ctxt->b == 0xe2 || test_cc(ctxt->b ^ 0x5, ctxt->eflags)))
b136801
-		jmp_rel(ctxt, ctxt->src.val);
b136801
+		rc = jmp_rel(ctxt, ctxt->src.val);
b136801
 
b136801
-	return X86EMUL_CONTINUE;
b136801
+	return rc;
b136801
 }
b136801
 
b136801
 static int em_jcxz(struct x86_emulate_ctxt *ctxt)
b136801
 {
b136801
+	int rc = X86EMUL_CONTINUE;
b136801
+
b136801
 	if (address_mask(ctxt, reg_read(ctxt, VCPU_REGS_RCX)) == 0)
b136801
-		jmp_rel(ctxt, ctxt->src.val);
b136801
+		rc = jmp_rel(ctxt, ctxt->src.val);
b136801
 
b136801
-	return X86EMUL_CONTINUE;
b136801
+	return rc;
b136801
 }
b136801
 
b136801
 static int em_in(struct x86_emulate_ctxt *ctxt)
b136801
@@ -4728,7 +4758,7 @@ special_insn:
b136801
 		break;
b136801
 	case 0x70 ... 0x7f: /* jcc (short) */
b136801
 		if (test_cc(ctxt->b, ctxt->eflags))
b136801
-			jmp_rel(ctxt, ctxt->src.val);
b136801
+			rc = jmp_rel(ctxt, ctxt->src.val);
b136801
 		break;
b136801
 	case 0x8d: /* lea r16/r32, m */
b136801
 		ctxt->dst.val = ctxt->src.addr.mem.ea;
b136801
@@ -4758,7 +4788,7 @@ special_insn:
b136801
 		break;
b136801
 	case 0xe9: /* jmp rel */
b136801
 	case 0xeb: /* jmp rel short */
b136801
-		jmp_rel(ctxt, ctxt->src.val);
b136801
+		rc = jmp_rel(ctxt, ctxt->src.val);
b136801
 		ctxt->dst.type = OP_NONE; /* Disable writeback. */
b136801
 		break;
b136801
 	case 0xf4:              /* hlt */
b136801
@@ -4881,7 +4911,7 @@ twobyte_insn:
b136801
 		break;
b136801
 	case 0x80 ... 0x8f: /* jnz rel, etc*/
b136801
 		if (test_cc(ctxt->b, ctxt->eflags))
b136801
-			jmp_rel(ctxt, ctxt->src.val);
b136801
+			rc = jmp_rel(ctxt, ctxt->src.val);
b136801
 		break;
b136801
 	case 0x90 ... 0x9f:     /* setcc r/m8 */
b136801
 		ctxt->dst.val = test_cc(ctxt->b, ctxt->eflags);
b136801
-- 
b136801
1.9.3
b136801