All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] Add segment limit checks to emulator
@ 2010-07-07 21:23 Mohammed Gamal
  2010-07-07 21:23 ` [RFC PATCH 1/3] Add helper methods to get segment limits Mohammed Gamal
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Mohammed Gamal @ 2010-07-07 21:23 UTC (permalink / raw)
  To: avi; +Cc: mtosatti, kvm, Mohammed Gamal

After some conversation with Avi concerning why unreal mode has been seen to work
with KVM on Intel. It clears out the scenario is caused as follows:

- guest enters big real mode
- kvm squashes limit to 64k-1
- guest executes instructions with offset > 64k
- cpu issues #GP due to limit violation
- kvm handle_rmode_exception() -> emulator
- emulator ignores limit, emulates instruction

With these applied I am getting vmentry failures with SeaBIOS and
gPXE. So it's needless to say that these patches are not meant for merging!

Mohammed Gamal (3):
  Add helper methods to get segment limits
  x86 emulator: Add cs_base() helper
  x86 emulator: Add segment limit checks and helper functions

 arch/x86/include/asm/kvm_emulate.h |    1 +
 arch/x86/include/asm/kvm_host.h    |    1 +
 arch/x86/kvm/emulate.c             |  123 +++++++++++++++++++++++++++++-------
 arch/x86/kvm/svm.c                 |    8 +++
 arch/x86/kvm/vmx.c                 |    8 +++
 arch/x86/kvm/x86.c                 |   12 ++++
 6 files changed, 130 insertions(+), 23 deletions(-)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC PATCH 1/3] Add helper methods to get segment limits
  2010-07-07 21:23 [RFC PATCH 0/3] Add segment limit checks to emulator Mohammed Gamal
@ 2010-07-07 21:23 ` Mohammed Gamal
  2010-07-07 21:23 ` [RFC PATCH 2/3] x86 emulator: Add cs_base() helper Mohammed Gamal
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mohammed Gamal @ 2010-07-07 21:23 UTC (permalink / raw)
  To: avi; +Cc: mtosatti, kvm, Mohammed Gamal

This adds helper methods to get segment limits for kvm_x86_ops and x86_emulate_ops. Hooks are added for SVM and VMX

Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
---
 arch/x86/include/asm/kvm_emulate.h |    1 +
 arch/x86/include/asm/kvm_host.h    |    1 +
 arch/x86/kvm/svm.c                 |    8 ++++++++
 arch/x86/kvm/vmx.c                 |    8 ++++++++
 arch/x86/kvm/x86.c                 |   12 ++++++++++++
 5 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 51cfd73..ce90048 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -138,6 +138,7 @@ struct x86_emulate_ops {
 	u16 (*get_segment_selector)(int seg, struct kvm_vcpu *vcpu);
 	void (*set_segment_selector)(u16 sel, int seg, struct kvm_vcpu *vcpu);
 	unsigned long (*get_cached_segment_base)(int seg, struct kvm_vcpu *vcpu);
+	u32 (*get_cached_segment_limit)(int seg, struct kvm_vcpu *vcpu);
 	void (*get_gdt)(struct desc_ptr *dt, struct kvm_vcpu *vcpu);
 	ulong (*get_cr)(int cr, struct kvm_vcpu *vcpu);
 	int (*set_cr)(int cr, ulong val, struct kvm_vcpu *vcpu);
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 502e53f..e32efc4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -468,6 +468,7 @@ struct kvm_x86_ops {
 	int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata);
 	int (*set_msr)(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
 	u64 (*get_segment_base)(struct kvm_vcpu *vcpu, int seg);
+	u32 (*get_segment_limit)(struct kvm_vcpu *vcpu, int seg);
 	void (*get_segment)(struct kvm_vcpu *vcpu,
 			    struct kvm_segment *var, int seg);
 	int (*get_cpl)(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 56c9b6b..504761d 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1038,6 +1038,13 @@ static u64 svm_get_segment_base(struct kvm_vcpu *vcpu, int seg)
 	return s->base;
 }
 
+static u32 svm_get_segment_limit(struct kvm_vcpu *vcpu, int seg)
+{
+	struct vmcb_seg *s = svm_seg(vcpu, seg);
+
+	return s->limit;
+}
+
 static void svm_get_segment(struct kvm_vcpu *vcpu,
 			    struct kvm_segment *var, int seg)
 {
@@ -3461,6 +3468,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.get_msr = svm_get_msr,
 	.set_msr = svm_set_msr,
 	.get_segment_base = svm_get_segment_base,
+	.get_segment_limit = svm_get_segment_limit,
 	.get_segment = svm_get_segment,
 	.set_segment = svm_set_segment,
 	.get_cpl = svm_get_cpl,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ebaaeaf..c9c14da 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2004,6 +2004,13 @@ static u64 vmx_get_segment_base(struct kvm_vcpu *vcpu, int seg)
 	return vmcs_readl(sf->base);
 }
 
+static u32 vmx_get_segment_limit(struct kvm_vcpu *vcpu, int seg)
+{
+	struct kvm_vmx_segment_field *sf = &kvm_vmx_segment_fields[seg];
+
+	return vmcs_read32(sf->limit);
+}
+
 static void vmx_get_segment(struct kvm_vcpu *vcpu,
 			    struct kvm_segment *var, int seg)
 {
@@ -4307,6 +4314,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.get_msr = vmx_get_msr,
 	.set_msr = vmx_set_msr,
 	.get_segment_base = vmx_get_segment_base,
+	.get_segment_limit = vmx_get_segment_limit,
 	.get_segment = vmx_get_segment,
 	.set_segment = vmx_set_segment,
 	.get_cpl = vmx_get_cpl,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7070b41..6a6aa92 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3674,6 +3674,11 @@ static unsigned long get_segment_base(struct kvm_vcpu *vcpu, int seg)
 	return kvm_x86_ops->get_segment_base(vcpu, seg);
 }
 
+static u32 get_segment_limit (struct kvm_vcpu *vcpu, int seg)
+{
+	return kvm_x86_ops->get_segment_limit(vcpu, seg);
+}
+
 int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)
 {
 	kvm_mmu_invlpg(vcpu, address);
@@ -3790,6 +3795,12 @@ static unsigned long emulator_get_cached_segment_base(int seg,
 	return get_segment_base(vcpu, seg);
 }
 
+static u32 emulate_get_cached_segment_limit(int seg,
+					    struct kvm_vcpu *vcpu)
+{
+	return get_segment_limit(vcpu, seg);
+}
+
 static bool emulator_get_cached_descriptor(struct desc_struct *desc, int seg,
 					   struct kvm_vcpu *vcpu)
 {
@@ -3876,6 +3887,7 @@ static struct x86_emulate_ops emulate_ops = {
 	.get_segment_selector = emulator_get_segment_selector,
 	.set_segment_selector = emulator_set_segment_selector,
 	.get_cached_segment_base = emulator_get_cached_segment_base,
+	.get_cached_segment_limit = emulate_get_cached_segment_limit,
 	.get_gdt             = emulator_get_gdt,
 	.get_cr              = emulator_get_cr,
 	.set_cr              = emulator_set_cr,
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 2/3] x86 emulator: Add cs_base() helper
  2010-07-07 21:23 [RFC PATCH 0/3] Add segment limit checks to emulator Mohammed Gamal
  2010-07-07 21:23 ` [RFC PATCH 1/3] Add helper methods to get segment limits Mohammed Gamal
@ 2010-07-07 21:23 ` Mohammed Gamal
  2010-07-07 21:23 ` [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions Mohammed Gamal
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mohammed Gamal @ 2010-07-07 21:23 UTC (permalink / raw)
  To: avi; +Cc: mtosatti, kvm, Mohammed Gamal

Adds a cs_base() helper for consistency with other emulator functions

Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
---
 arch/x86/kvm/emulate.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e8bdddc..f40479a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -642,6 +642,12 @@ static unsigned long seg_override_base(struct x86_emulate_ctxt *ctxt,
 	return seg_base(ctxt, ops, c->seg_override);
 }
 
+static unsigned long cs_base(struct x86_emulate_ctxt *ctxt,
+			     struct x86_emulate_ops *ops)
+{
+	return seg_base(ctxt, ops, VCPU_SREG_CS);
+}
+
 static unsigned long es_base(struct x86_emulate_ctxt *ctxt,
 			     struct x86_emulate_ops *ops)
 {
@@ -975,7 +981,7 @@ x86_decode_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops)
 
 	c->eip = ctxt->eip;
 	c->fetch.start = c->fetch.end = c->eip;
-	ctxt->cs_base = seg_base(ctxt, ops, VCPU_SREG_CS);
+	ctxt->cs_base = cs_base(ctxt, ops);
 
 	switch (mode) {
 	case X86EMUL_MODE_REAL:
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions
  2010-07-07 21:23 [RFC PATCH 0/3] Add segment limit checks to emulator Mohammed Gamal
  2010-07-07 21:23 ` [RFC PATCH 1/3] Add helper methods to get segment limits Mohammed Gamal
  2010-07-07 21:23 ` [RFC PATCH 2/3] x86 emulator: Add cs_base() helper Mohammed Gamal
@ 2010-07-07 21:23 ` Mohammed Gamal
  2010-07-08  8:01   ` Avi Kivity
  2010-07-08  8:05 ` [RFC PATCH 0/3] Add segment limit checks to emulator Stefan Hajnoczi
  2010-07-08  8:07 ` Avi Kivity
  4 siblings, 1 reply; 7+ messages in thread
From: Mohammed Gamal @ 2010-07-07 21:23 UTC (permalink / raw)
  To: avi; +Cc: mtosatti, kvm, Mohammed Gamal

This patch adds segment limit checks to the x86 emulator, in addition to some
helper functions and changes to the return values of emulate_push to accomodate
the new checks.

Signed-off-by: Mohammed Gamal <m.gamal005@gmail.com>
---
 arch/x86/kvm/emulate.c |  115 ++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 93 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f40479a..1bd34dc 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -632,6 +632,15 @@ static unsigned long seg_base(struct x86_emulate_ctxt *ctxt,
 	return ops->get_cached_segment_base(seg, ctxt->vcpu);
 }
 
+static u32 seg_limit(struct x86_emulate_ctxt *ctxt,
+		     struct x86_emulate_ops *ops, int seg)
+{
+	if (ctxt->mode == X86EMUL_MODE_PROT64 && seg < VCPU_SREG_FS)
+		return 0;
+
+	return ops->get_cached_segment_limit(seg, ctxt->vcpu);
+}
+
 static unsigned long seg_override_base(struct x86_emulate_ctxt *ctxt,
 				       struct x86_emulate_ops *ops,
 				       struct decode_cache *c)
@@ -660,6 +669,24 @@ static unsigned long ss_base(struct x86_emulate_ctxt *ctxt,
 	return seg_base(ctxt, ops, VCPU_SREG_SS);
 }
 
+static u32 cs_limit(struct x86_emulate_ctxt *ctxt,
+		    struct x86_emulate_ops *ops)
+{
+	return seg_limit(ctxt, ops, VCPU_SREG_CS);
+}
+
+static u32 es_limit(struct x86_emulate_ctxt *ctxt,
+		    struct x86_emulate_ops *ops)
+{
+	return seg_limit(ctxt, ops, VCPU_SREG_ES);
+}
+
+static u32 ss_limit(struct x86_emulate_ctxt *ctxt,
+		    struct x86_emulate_ops *ops)
+{
+	return seg_limit(ctxt, ops, VCPU_SREG_SS);
+}
+
 static void emulate_exception(struct x86_emulate_ctxt *ctxt, int vec,
 				      u32 error, bool valid)
 {
@@ -718,6 +745,11 @@ static int do_insn_fetch(struct x86_emulate_ctxt *ctxt,
 {
 	int rc;
 
+	if (eip + size > cs_base(ctxt, ops) + cs_limit(ctxt, ops)) {
+		emulate_gp(ctxt, 0);
+		return X86EMUL_PROPAGATE_FAULT;
+	}
+
 	/* x86 instructions are limited to 15 bytes. */
 	if (eip + size - ctxt->eip > 15)
 		return X86EMUL_UNHANDLEABLE;
@@ -1202,6 +1234,10 @@ done_prefixes:
 		c->src.ptr = (unsigned long *)
 			register_address(c,  seg_override_base(ctxt, ops, c),
 					 c->regs[VCPU_REGS_RSI]);
+		if (c->src.ptr > (unsigned long *) (es_base(ctxt, ops) + es_limit(ctxt, ops))) {
+			emulate_gp(ctxt, 0);
+			return X86EMUL_PROPAGATE_FAULT;
+		}
 		c->src.val = 0;
 		break;
 	case SrcImmFAddr:
@@ -1298,6 +1334,10 @@ done_prefixes:
 		c->dst.ptr = (unsigned long *)
 			register_address(c, es_base(ctxt, ops),
 					 c->regs[VCPU_REGS_RDI]);
+		if (c->dst.ptr > (unsigned long *) (es_base(ctxt, ops) + es_limit(ctxt, ops))) {
+			emulate_gp(ctxt, 0);
+			return X86EMUL_PROPAGATE_FAULT;
+		}
 		c->dst.val = 0;
 		break;
 	}
@@ -1617,7 +1657,7 @@ static inline int writeback(struct x86_emulate_ctxt *ctxt,
 	return X86EMUL_CONTINUE;
 }
 
-static inline void emulate_push(struct x86_emulate_ctxt *ctxt,
+static inline int emulate_push(struct x86_emulate_ctxt *ctxt,
 				struct x86_emulate_ops *ops)
 {
 	struct decode_cache *c = &ctxt->decode;
@@ -1628,6 +1668,12 @@ static inline void emulate_push(struct x86_emulate_ctxt *ctxt,
 	register_address_increment(c, &c->regs[VCPU_REGS_RSP], -c->op_bytes);
 	c->dst.ptr = (void *) register_address(c, ss_base(ctxt, ops),
 					       c->regs[VCPU_REGS_RSP]);
+	if (c->dst.ptr > (unsigned long *) (ss_base(ctxt, ops) + ss_limit(ctxt, ops))) {
+		emulate_gp(ctxt, 0);
+		return X86EMUL_PROPAGATE_FAULT;
+	}
+
+	return X86EMUL_CONTINUE;
 }
 
 static int emulate_pop(struct x86_emulate_ctxt *ctxt,
@@ -1635,11 +1681,15 @@ static int emulate_pop(struct x86_emulate_ctxt *ctxt,
 		       void *dest, int len)
 {
 	struct decode_cache *c = &ctxt->decode;
+	unsigned long reg_addr = register_address(c, ss_base(ctxt, ops), c->regs[VCPU_REGS_RSP]);
 	int rc;
 
-	rc = read_emulated(ctxt, ops, register_address(c, ss_base(ctxt, ops),
-						       c->regs[VCPU_REGS_RSP]),
-			   dest, len);
+	if (reg_addr > ss_base(ctxt, ops) + ss_limit(ctxt, ops)) {
+		emulate_gp(ctxt, 0);
+		return X86EMUL_PROPAGATE_FAULT;
+	}
+
+	rc = read_emulated(ctxt, ops, reg_addr, dest, len);
 	if (rc != X86EMUL_CONTINUE)
 		return rc;
 
@@ -1690,14 +1740,14 @@ static int emulate_popf(struct x86_emulate_ctxt *ctxt,
 	return rc;
 }
 
-static void emulate_push_sreg(struct x86_emulate_ctxt *ctxt,
+static int emulate_push_sreg(struct x86_emulate_ctxt *ctxt,
 			      struct x86_emulate_ops *ops, int seg)
 {
 	struct decode_cache *c = &ctxt->decode;
 
 	c->src.val = ops->get_segment_selector(seg, ctxt->vcpu);
 
-	emulate_push(ctxt, ops);
+	return emulate_push(ctxt, ops);
 }
 
 static int emulate_pop_sreg(struct x86_emulate_ctxt *ctxt,
@@ -1727,7 +1777,10 @@ static int emulate_pusha(struct x86_emulate_ctxt *ctxt,
 		(reg == VCPU_REGS_RSP) ?
 		(c->src.val = old_esp) : (c->src.val = c->regs[reg]);
 
-		emulate_push(ctxt, ops);
+		rc = emulate_push(ctxt, ops);
+
+		if (rc != X86EMUL_CONTINUE)
+			return rc;
 
 		rc = writeback(ctxt, ops);
 		if (rc != X86EMUL_CONTINUE)
@@ -1839,15 +1892,13 @@ static inline int emulate_grp45(struct x86_emulate_ctxt *ctxt,
 		old_eip = c->eip;
 		c->eip = c->src.val;
 		c->src.val = old_eip;
-		emulate_push(ctxt, ops);
-		break;
+		return emulate_push(ctxt, ops);
 	}
 	case 4: /* jmp abs */
 		c->eip = c->src.val;
 		break;
 	case 6:	/* push */
-		emulate_push(ctxt, ops);
-		break;
+		return emulate_push(ctxt, ops);
 	}
 	return X86EMUL_CONTINUE;
 }
@@ -2503,7 +2554,7 @@ static int emulator_do_task_switch(struct x86_emulate_ctxt *ctxt,
 		c->op_bytes = c->ad_bytes = (next_tss_desc.type & 8) ? 4 : 2;
 		c->lock_prefix = 0;
 		c->src.val = (unsigned long) error_code;
-		emulate_push(ctxt, ops);
+		ret = emulate_push(ctxt, ops);
 	}
 
 	return ret;
@@ -2636,7 +2687,9 @@ special_insn:
 		emulate_2op_SrcV("add", c->src, c->dst, ctxt->eflags);
 		break;
 	case 0x06:		/* push es */
-		emulate_push_sreg(ctxt, ops, VCPU_SREG_ES);
+		rc = emulate_push_sreg(ctxt, ops, VCPU_SREG_ES);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x07:		/* pop es */
 		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_ES);
@@ -2648,14 +2701,18 @@ special_insn:
 		emulate_2op_SrcV("or", c->src, c->dst, ctxt->eflags);
 		break;
 	case 0x0e:		/* push cs */
-		emulate_push_sreg(ctxt, ops, VCPU_SREG_CS);
+		rc = emulate_push_sreg(ctxt, ops, VCPU_SREG_CS);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x10 ... 0x15:
 	      adc:		/* adc */
 		emulate_2op_SrcV("adc", c->src, c->dst, ctxt->eflags);
 		break;
 	case 0x16:		/* push ss */
-		emulate_push_sreg(ctxt, ops, VCPU_SREG_SS);
+		rc = emulate_push_sreg(ctxt, ops, VCPU_SREG_SS);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x17:		/* pop ss */
 		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_SS);
@@ -2667,7 +2724,9 @@ special_insn:
 		emulate_2op_SrcV("sbb", c->src, c->dst, ctxt->eflags);
 		break;
 	case 0x1e:		/* push ds */
-		emulate_push_sreg(ctxt, ops, VCPU_SREG_DS);
+		rc = emulate_push_sreg(ctxt, ops, VCPU_SREG_DS);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x1f:		/* pop ds */
 		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_DS);
@@ -2697,7 +2756,9 @@ special_insn:
 		emulate_1op("dec", c->dst, ctxt->eflags);
 		break;
 	case 0x50 ... 0x57:  /* push reg */
-		emulate_push(ctxt, ops);
+		rc = emulate_push(ctxt, ops);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x58 ... 0x5f: /* pop reg */
 	pop_instruction:
@@ -2722,7 +2783,9 @@ special_insn:
 		break;
 	case 0x68: /* push imm */
 	case 0x6a: /* push imm8 */
-		emulate_push(ctxt, ops);
+		rc = emulate_push(ctxt, ops);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x6c:		/* insb */
 	case 0x6d:		/* insw/insd */
@@ -2850,7 +2913,9 @@ special_insn:
 		goto xchg;
 	case 0x9c: /* pushf */
 		c->src.val =  (unsigned long) ctxt->eflags;
-		emulate_push(ctxt, ops);
+		rc = emulate_push(ctxt, ops);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0x9d: /* popf */
 		c->dst.type = OP_REG;
@@ -2920,7 +2985,9 @@ special_insn:
 		long int rel = c->src.val;
 		c->src.val = (unsigned long) c->eip;
 		jmp_rel(c, rel);
-		emulate_push(ctxt, ops);
+		rc = emulate_push(ctxt, ops);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	}
 	case 0xe9: /* jmp rel */
@@ -3245,7 +3312,9 @@ twobyte_insn:
 		c->dst.type = OP_NONE;
 		break;
 	case 0xa0:	  /* push fs */
-		emulate_push_sreg(ctxt, ops, VCPU_SREG_FS);
+		rc = emulate_push_sreg(ctxt, ops, VCPU_SREG_FS);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0xa1:	 /* pop fs */
 		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_FS);
@@ -3264,7 +3333,9 @@ twobyte_insn:
 		emulate_2op_cl("shld", c->src2, c->src, c->dst, ctxt->eflags);
 		break;
 	case 0xa8:	/* push gs */
-		emulate_push_sreg(ctxt, ops, VCPU_SREG_GS);
+		rc = emulate_push_sreg(ctxt, ops, VCPU_SREG_GS);
+		if (rc != X86EMUL_CONTINUE)
+			goto done;
 		break;
 	case 0xa9:	/* pop gs */
 		rc = emulate_pop_sreg(ctxt, ops, VCPU_SREG_GS);
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions
  2010-07-07 21:23 ` [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions Mohammed Gamal
@ 2010-07-08  8:01   ` Avi Kivity
  0 siblings, 0 replies; 7+ messages in thread
From: Avi Kivity @ 2010-07-08  8:01 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: mtosatti, kvm

On 07/08/2010 12:23 AM, Mohammed Gamal wrote:
> This patch adds segment limit checks to the x86 emulator, in addition to some
> helper functions and changes to the return values of emulate_push to accomodate
> the new checks.
>
>
>
> +static u32 seg_limit(struct x86_emulate_ctxt *ctxt,
> +		     struct x86_emulate_ops *ops, int seg)
> +{
> +	if (ctxt->mode == X86EMUL_MODE_PROT64&&  seg<  VCPU_SREG_FS)
>    

Why the check on VCPU_SREG_FS?  There are no limits in long mode (well 
there's some AMD thing that does allow them).

> +		return 0;
>    

better to return -1ULL, that indicates no practical limit.

> +
> +	return ops->get_cached_segment_limit(seg, ctxt->vcpu);
> +}
> +
>    

>   static void emulate_exception(struct x86_emulate_ctxt *ctxt, int vec,
>   				      u32 error, bool valid)
>   {
> @@ -718,6 +745,11 @@ static int do_insn_fetch(struct x86_emulate_ctxt *ctxt,
>   {
>   	int rc;
>
> +	if (eip + size>  cs_base(ctxt, ops) + cs_limit(ctxt, ops)) {
>    

This can wrap around, for example if cs.base=0xf0000000, cs.limit=0x2000000.

Comparing eip - cs_base + size < cs_limit works around that.

> @@ -1202,6 +1234,10 @@ done_prefixes:
>   		c->src.ptr = (unsigned long *)
>   			register_address(c,  seg_override_base(ctxt, ops, c),
>   					 c->regs[VCPU_REGS_RSI]);
> +		if (c->src.ptr>  (unsigned long *) (es_base(ctxt, ops) + es_limit(ctxt, ops))) {
> +			emulate_gp(ctxt, 0);
> +			return X86EMUL_PROPAGATE_FAULT;
>    

Need to take into account the size fetched from src.ptr.  For data 
segments, there are expand-down segments which modify the check.  See 
SDM 5.3, "Limit Checking".



-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 0/3] Add segment limit checks to emulator
  2010-07-07 21:23 [RFC PATCH 0/3] Add segment limit checks to emulator Mohammed Gamal
                   ` (2 preceding siblings ...)
  2010-07-07 21:23 ` [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions Mohammed Gamal
@ 2010-07-08  8:05 ` Stefan Hajnoczi
  2010-07-08  8:07 ` Avi Kivity
  4 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2010-07-08  8:05 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: avi, mtosatti, kvm

On Wed, Jul 7, 2010 at 10:23 PM, Mohammed Gamal <m.gamal005@gmail.com> wrote:
> After some conversation with Avi concerning why unreal mode has been seen to work
> with KVM on Intel. It clears out the scenario is caused as follows:
>
> - guest enters big real mode
> - kvm squashes limit to 64k-1
> - guest executes instructions with offset > 64k
> - cpu issues #GP due to limit violation
> - kvm handle_rmode_exception() -> emulator
> - emulator ignores limit, emulates instruction
>
> With these applied I am getting vmentry failures with SeaBIOS and
> gPXE. So it's needless to say that these patches are not meant for merging!

Thanks for testing with gPXE and please don't hesitate to discuss
issues you hit with gPXE on gpxe-devel@etherboot.org.  I look forward
to your big real mode work :).

> Mohammed Gamal (3):
>  Add helper methods to get segment limits
>  x86 emulator: Add cs_base() helper
>  x86 emulator: Add segment limit checks and helper functions
>
>  arch/x86/include/asm/kvm_emulate.h |    1 +
>  arch/x86/include/asm/kvm_host.h    |    1 +
>  arch/x86/kvm/emulate.c             |  123 +++++++++++++++++++++++++++++-------
>  arch/x86/kvm/svm.c                 |    8 +++
>  arch/x86/kvm/vmx.c                 |    8 +++
>  arch/x86/kvm/x86.c                 |   12 ++++
>  6 files changed, 130 insertions(+), 23 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Stefan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH 0/3] Add segment limit checks to emulator
  2010-07-07 21:23 [RFC PATCH 0/3] Add segment limit checks to emulator Mohammed Gamal
                   ` (3 preceding siblings ...)
  2010-07-08  8:05 ` [RFC PATCH 0/3] Add segment limit checks to emulator Stefan Hajnoczi
@ 2010-07-08  8:07 ` Avi Kivity
  4 siblings, 0 replies; 7+ messages in thread
From: Avi Kivity @ 2010-07-08  8:07 UTC (permalink / raw)
  To: Mohammed Gamal; +Cc: mtosatti, kvm

On 07/08/2010 12:23 AM, Mohammed Gamal wrote:
> After some conversation with Avi concerning why unreal mode has been seen to work
> with KVM on Intel. It clears out the scenario is caused as follows:
>
> - guest enters big real mode
> - kvm squashes limit to 64k-1
> - guest executes instructions with offset>  64k
> - cpu issues #GP due to limit violation
> - kvm handle_rmode_exception() ->  emulator
> - emulator ignores limit, emulates instruction
>
> With these applied I am getting vmentry failures with SeaBIOS and
> gPXE. So it's needless to say that these patches are not meant for merging!
>    

btw, we'll want unit tests for those.  Access just below the limit, 
access that's partially within and partially outside the limit, with 
base=0 and base!=0, in 16, 32 and 64 bit modes.

IIRC the emulator only builds in 64 bits.  I'll see what it takes to 
make it build and run on 32 bits.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-07-08  8:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-07 21:23 [RFC PATCH 0/3] Add segment limit checks to emulator Mohammed Gamal
2010-07-07 21:23 ` [RFC PATCH 1/3] Add helper methods to get segment limits Mohammed Gamal
2010-07-07 21:23 ` [RFC PATCH 2/3] x86 emulator: Add cs_base() helper Mohammed Gamal
2010-07-07 21:23 ` [RFC PATCH 3/3] x86 emulator: Add segment limit checks and helper functions Mohammed Gamal
2010-07-08  8:01   ` Avi Kivity
2010-07-08  8:05 ` [RFC PATCH 0/3] Add segment limit checks to emulator Stefan Hajnoczi
2010-07-08  8:07 ` Avi Kivity

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.