* [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.