| Message ID | 1416411793-22244-3-git-send-email-namit@cs.technion.ac.il (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/11/2014 16:43, Nadav Amit wrote: > When performing segmented-read/write in the emulator for stack operations, it > ignores the stack size, and uses the ad_bytes as indication for the pointer > size. As a result, a wrong address may be accessed. > > To fix this behavior, we can remove the masking of address in __linearize and > perform it beforehand. It is already done for the operands (so currently it is > inefficiently done twice). It is missing in two cases: > 1. When using rip_relative > 2. On fetch_bit_operand that changes the address. > > This patch masks the address on these two occassions, and removes the masking > from __linearize. > > Note that it does not mask EIP during fetch. In protected/legacy mode code > fetch when RIP >= 2^32 should result in #GP and not wrap-around. Since we make > limit checks within __linearize, this is the expected behavior. Partial revert of commit 518547b32ab4 (KVM: x86: Emulator does not calculate address correctly, 2014-09-30). Paolo > Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> > --- > arch/x86/kvm/emulate.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 5d47714..1317560 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -665,8 +665,7 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > u16 sel; > unsigned cpl; > > - la = seg_base(ctxt, addr.seg) + > - (fetch || ctxt->ad_bytes == 8 ? addr.ea : (u32)addr.ea); > + la = seg_base(ctxt, addr.seg) + addr.ea; > *max_size = 0; > switch (ctxt->mode) { > case X86EMUL_MODE_PROT64: > @@ -1289,7 +1288,8 @@ static void fetch_bit_operand(struct x86_emulate_ctxt *ctxt) > else > sv = (s64)ctxt->src.val & (s64)mask; > > - ctxt->dst.addr.mem.ea += (sv >> 3); > + ctxt->dst.addr.mem.ea = address_mask(ctxt, > + ctxt->dst.addr.mem.ea + (sv >> 3)); > } > > /* only subword offset */ > @@ -4638,7 +4638,8 @@ done_prefixes: > rc = decode_operand(ctxt, &ctxt->dst, (ctxt->d >> DstShift) & OpMask); > > if (ctxt->rip_relative) > - ctxt->memopp->addr.mem.ea += ctxt->_eip; > + ctxt->memopp->addr.mem.ea = address_mask(ctxt, > + ctxt->memopp->addr.mem.ea + ctxt->_eip); > > done: > return (rc != X86EMUL_CONTINUE) ? EMULATION_FAILED : EMULATION_OK; > -- 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
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 5d47714..1317560 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -665,8 +665,7 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, u16 sel; unsigned cpl; - la = seg_base(ctxt, addr.seg) + - (fetch || ctxt->ad_bytes == 8 ? addr.ea : (u32)addr.ea); + la = seg_base(ctxt, addr.seg) + addr.ea; *max_size = 0; switch (ctxt->mode) { case X86EMUL_MODE_PROT64: @@ -1289,7 +1288,8 @@ static void fetch_bit_operand(struct x86_emulate_ctxt *ctxt) else sv = (s64)ctxt->src.val & (s64)mask; - ctxt->dst.addr.mem.ea += (sv >> 3); + ctxt->dst.addr.mem.ea = address_mask(ctxt, + ctxt->dst.addr.mem.ea + (sv >> 3)); } /* only subword offset */ @@ -4638,7 +4638,8 @@ done_prefixes: rc = decode_operand(ctxt, &ctxt->dst, (ctxt->d >> DstShift) & OpMask); if (ctxt->rip_relative) - ctxt->memopp->addr.mem.ea += ctxt->_eip; + ctxt->memopp->addr.mem.ea = address_mask(ctxt, + ctxt->memopp->addr.mem.ea + ctxt->_eip); done: return (rc != X86EMUL_CONTINUE) ? EMULATION_FAILED : EMULATION_OK;
When performing segmented-read/write in the emulator for stack operations, it ignores the stack size, and uses the ad_bytes as indication for the pointer size. As a result, a wrong address may be accessed. To fix this behavior, we can remove the masking of address in __linearize and perform it beforehand. It is already done for the operands (so currently it is inefficiently done twice). It is missing in two cases: 1. When using rip_relative 2. On fetch_bit_operand that changes the address. This patch masks the address on these two occassions, and removes the masking from __linearize. Note that it does not mask EIP during fetch. In protected/legacy mode code fetch when RIP >= 2^32 should result in #GP and not wrap-around. Since we make limit checks within __linearize, this is the expected behavior. Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/kvm/emulate.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)