| Message ID | 56C4AF7D02000078000D3490@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17/02/16 16:35, Jan Beulich wrote: > All branches need to #GP when their target violates the segment limit > (in 16- and 32-bit modes) or is non-canonical (in 64-bit mode). For > near branches facilitate this via a zero-byte instruction fetch from > the target address (resulting in address translation and validation > without an actual read from memory), while far branches get dealt with > by breaking up the segment register loading into a read-and-validate > part and a write one. The latter at once allows correcting some > ordering issues in how the individual emulation steps get carried out: > Before updating machine state, all exceptions unrelated to that state > updating should have got raised (i.e. the only ones possibly resulting > in partly updated state are faulting memory writes [pushes]). > > Note that while not immediately needed here, write and distinct read > emulation routines get updated to deal with zero byte accesses too, for > overall consistency. > > Reported-by: 刘令 <liuling-it@360.cn> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> On 17.02.16 at 17:35, <JBeulich@suse.com> wrote: > All branches need to #GP when their target violates the segment limit > (in 16- and 32-bit modes) or is non-canonical (in 64-bit mode). For > near branches facilitate this via a zero-byte instruction fetch from > the target address (resulting in address translation and validation > without an actual read from memory), while far branches get dealt with > by breaking up the segment register loading into a read-and-validate > part and a write one. The latter at once allows correcting some > ordering issues in how the individual emulation steps get carried out: > Before updating machine state, all exceptions unrelated to that state > updating should have got raised (i.e. the only ones possibly resulting > in partly updated state are faulting memory writes [pushes]). > > Note that while not immediately needed here, write and distinct read > emulation routines get updated to deal with zero byte accesses too, for > overall consistency. > > Reported-by: 刘令 <liuling-it@360.cn> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/tools/tests/x86_emulator/x86_emulate.c > +++ b/tools/tests/x86_emulator/x86_emulate.c > @@ -8,6 +8,8 @@ > > typedef bool bool_t; > > +#define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63)) > + > #define BUG() abort() > #define ASSERT assert > > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -745,7 +745,7 @@ static int __hvmemul_read( > > rc = hvmemul_virtual_to_linear( > seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr); > - if ( rc != X86EMUL_OKAY ) > + if ( rc != X86EMUL_OKAY || !bytes ) > return rc; > if ( ((access_type != hvm_access_insn_fetch > ? vio->mmio_access.read_access > @@ -811,13 +811,17 @@ static int hvmemul_insn_fetch( > container_of(ctxt, struct hvm_emulate_ctxt, ctxt); > unsigned int insn_off = offset - hvmemul_ctxt->insn_buf_eip; > > - /* Fall back if requested bytes are not in the prefetch cache. */ > - if ( unlikely((insn_off + bytes) > hvmemul_ctxt->insn_buf_bytes) ) > + /* > + * Fall back if requested bytes are not in the prefetch cache. > + * But always perform the (fake) read when bytes == 0. > + */ > + if ( !bytes || > + unlikely((insn_off + bytes) > hvmemul_ctxt->insn_buf_bytes) ) > { > int rc = __hvmemul_read(seg, offset, p_data, bytes, > hvm_access_insn_fetch, hvmemul_ctxt); > > - if ( rc == X86EMUL_OKAY ) > + if ( rc == X86EMUL_OKAY && bytes ) > { > ASSERT(insn_off + bytes <= sizeof(hvmemul_ctxt->insn_buf)); > memcpy(&hvmemul_ctxt->insn_buf[insn_off], p_data, bytes); > @@ -849,7 +853,7 @@ static int hvmemul_write( > > rc = hvmemul_virtual_to_linear( > seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); > - if ( rc != X86EMUL_OKAY ) > + if ( rc != X86EMUL_OKAY || !bytes ) > return rc; > > if ( vio->mmio_access.write_access && > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3685,7 +3685,7 @@ int hvm_virtual_to_linear_addr( > * Certain of them are not done in native real mode anyway. > */ > addr = (uint32_t)(addr + reg->base); > - last_byte = (uint32_t)addr + bytes - 1; > + last_byte = (uint32_t)addr + bytes - !!bytes; > if ( last_byte < addr ) > return 0; > } > @@ -3709,7 +3709,7 @@ int hvm_virtual_to_linear_addr( > break; > } > > - last_byte = (uint32_t)offset + bytes - 1; > + last_byte = (uint32_t)offset + bytes - !!bytes; > > /* Is this a grows-down data segment? Special limit check if so. */ > if ( (reg->attr.fields.type & 0xc) == 0x4 ) > @@ -3740,7 +3740,7 @@ int hvm_virtual_to_linear_addr( > if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) ) > addr += reg->base; > > - last_byte = addr + bytes - 1; > + last_byte = addr + bytes - !!bytes; > if ( !is_canonical_address(addr) || last_byte < addr || > !is_canonical_address(last_byte) ) > return 0; > --- a/xen/arch/x86/mm/shadow/common.c > +++ b/xen/arch/x86/mm/shadow/common.c > @@ -157,7 +157,7 @@ hvm_read(enum x86_segment seg, > > rc = hvm_translate_linear_addr( > seg, offset, bytes, access_type, sh_ctxt, &addr); > - if ( rc ) > + if ( rc || !bytes ) > return rc; > > if ( access_type == hvm_access_insn_fetch ) > @@ -241,7 +241,7 @@ hvm_emulate_write(enum x86_segment seg, > > rc = hvm_translate_linear_addr( > seg, offset, bytes, hvm_access_write, sh_ctxt, &addr); > - if ( rc ) > + if ( rc || !bytes ) > return rc; > > return v->arch.paging.mode->shadow.x86_emulate_write( > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -5126,10 +5126,11 @@ static int ptwr_emulated_read( > unsigned int bytes, > struct x86_emulate_ctxt *ctxt) > { > - unsigned int rc; > + unsigned int rc = bytes; > unsigned long addr = offset; > > - if ( (rc = copy_from_user(p_data, (void *)addr, bytes)) != 0 ) > + if ( !__addr_ok(addr) || > + (rc = __copy_from_user(p_data, (void *)addr, bytes)) ) > { > propagate_page_fault(addr + bytes - rc, 0); /* read fault */ > return X86EMUL_EXCEPTION; > @@ -5278,7 +5279,7 @@ static int ptwr_emulated_write( > { > paddr_t val = 0; > > - if ( (bytes > sizeof(paddr_t)) || (bytes & (bytes -1)) ) > + if ( (bytes > sizeof(paddr_t)) || (bytes & (bytes - 1)) || !bytes ) > { > MEM_LOG("ptwr_emulate: bad write size (addr=%lx, bytes=%u)", > offset, bytes); > @@ -5394,7 +5395,8 @@ int mmio_ro_emulated_write( > struct mmio_ro_emulate_ctxt *mmio_ro_ctxt = ctxt->data; > > /* Only allow naturally-aligned stores at the original %cr2 address. */ > - if ( ((bytes | offset) & (bytes - 1)) || offset != mmio_ro_ctxt->cr2 ) > + if ( ((bytes | offset) & (bytes - 1)) || !bytes || > + offset != mmio_ro_ctxt->cr2 ) > { > MEM_LOG("mmio_ro_emulate: bad access (cr2=%lx, addr=%lx, > bytes=%u)", > mmio_ro_ctxt->cr2, offset, bytes); > @@ -5423,7 +5425,7 @@ int mmcfg_intercept_write( > * Only allow naturally-aligned stores no wider than 4 bytes to the > * original %cr2 address. > */ > - if ( ((bytes | offset) & (bytes - 1)) || bytes > 4 || > + if ( ((bytes | offset) & (bytes - 1)) || bytes > 4 || !bytes || > offset != mmio_ctxt->cr2 ) > { > MEM_LOG("mmcfg_intercept: bad write (cr2=%lx, addr=%lx, bytes=%u)", > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -642,14 +642,26 @@ do { > > #define jmp_rel(rel) \ > do { \ > - int _rel = (int)(rel); \ > - _regs.eip += _rel; \ > + unsigned long ip = _regs.eip + (int)(rel); \ > if ( op_bytes == 2 ) \ > - _regs.eip = (uint16_t)_regs.eip; \ > + ip = (uint16_t)ip; \ > else if ( !mode_64bit() ) \ > - _regs.eip = (uint32_t)_regs.eip; \ > + ip = (uint32_t)ip; \ > + rc = ops->insn_fetch(x86_seg_cs, ip, NULL, 0, ctxt); \ > + if ( rc ) goto done; \ > + _regs.eip = ip; \ > } while (0) > > +#define validate_far_branch(cs, ip) \ > + generate_exception_if(in_longmode(ctxt, ops) && (cs)->attr.fields.l \ > + ? !is_canonical_address(ip) \ > + : (ip) > (cs)->limit, EXC_GP, 0) > + > +#define commit_far_branch(cs, ip) ({ \ > + validate_far_branch(cs, ip); \ > + ops->write_segment(x86_seg_cs, cs, ctxt); \ > +}) > + > struct fpu_insn_ctxt { > uint8_t insn_bytes; > uint8_t exn_raised; > @@ -1099,29 +1111,30 @@ static int > realmode_load_seg( > enum x86_segment seg, > uint16_t sel, > + struct segment_register *sreg, > struct x86_emulate_ctxt *ctxt, > const struct x86_emulate_ops *ops) > { > - struct segment_register reg; > - int rc; > + int rc = ops->read_segment(seg, sreg, ctxt); > > - if ( (rc = ops->read_segment(seg, ®, ctxt)) != 0 ) > - return rc; > - > - reg.sel = sel; > - reg.base = (uint32_t)sel << 4; > + if ( !rc ) > + { > + sreg->sel = sel; > + sreg->base = (uint32_t)sel << 4; > + } > > - return ops->write_segment(seg, ®, ctxt); > + return rc; > } > > static int > protmode_load_seg( > enum x86_segment seg, > uint16_t sel, bool_t is_ret, > + struct segment_register *sreg, > struct x86_emulate_ctxt *ctxt, > const struct x86_emulate_ops *ops) > { > - struct segment_register desctab, ss, segr; > + struct segment_register desctab, ss; > struct { uint32_t a, b; } desc; > uint8_t dpl, rpl, cpl; > uint32_t new_desc_b, a_flag = 0x100; > @@ -1132,8 +1145,8 @@ protmode_load_seg( > { > if ( (seg == x86_seg_cs) || (seg == x86_seg_ss) ) > goto raise_exn; > - memset(&segr, 0, sizeof(segr)); > - return ops->write_segment(seg, &segr, ctxt); > + memset(sreg, 0, sizeof(*sreg)); > + return X86EMUL_OKAY; > } > > /* System segment descriptors must reside in the GDT. */ > @@ -1242,16 +1255,16 @@ protmode_load_seg( > desc.b |= a_flag; > > skip_accessed_flag: > - segr.base = (((desc.b << 0) & 0xff000000u) | > - ((desc.b << 16) & 0x00ff0000u) | > - ((desc.a >> 16) & 0x0000ffffu)); > - segr.attr.bytes = (((desc.b >> 8) & 0x00ffu) | > - ((desc.b >> 12) & 0x0f00u)); > - segr.limit = (desc.b & 0x000f0000u) | (desc.a & 0x0000ffffu); > - if ( segr.attr.fields.g ) > - segr.limit = (segr.limit << 12) | 0xfffu; > - segr.sel = sel; > - return ops->write_segment(seg, &segr, ctxt); > + sreg->base = (((desc.b << 0) & 0xff000000u) | > + ((desc.b << 16) & 0x00ff0000u) | > + ((desc.a >> 16) & 0x0000ffffu)); > + sreg->attr.bytes = (((desc.b >> 8) & 0x00ffu) | > + ((desc.b >> 12) & 0x0f00u)); > + sreg->limit = (desc.b & 0x000f0000u) | (desc.a & 0x0000ffffu); > + if ( sreg->attr.fields.g ) > + sreg->limit = (sreg->limit << 12) | 0xfffu; > + sreg->sel = sel; > + return X86EMUL_OKAY; > > raise_exn: > if ( ops->inject_hw_exception == NULL ) > @@ -1265,17 +1278,29 @@ static int > load_seg( > enum x86_segment seg, > uint16_t sel, bool_t is_ret, > + struct segment_register *sreg, > struct x86_emulate_ctxt *ctxt, > const struct x86_emulate_ops *ops) > { > + struct segment_register reg; > + int rc; > + > if ( (ops->read_segment == NULL) || > (ops->write_segment == NULL) ) > return X86EMUL_UNHANDLEABLE; > > + if ( !sreg ) > + sreg = ® > + > if ( in_protmode(ctxt, ops) ) > - return protmode_load_seg(seg, sel, is_ret, ctxt, ops); > + rc = protmode_load_seg(seg, sel, is_ret, sreg, ctxt, ops); > + else > + rc = realmode_load_seg(seg, sel, sreg, ctxt, ops); > + > + if ( !rc && sreg == ® ) > + rc = ops->write_segment(seg, sreg, ctxt); > > - return realmode_load_seg(seg, sel, ctxt, ops); > + return rc; > } > > void * > @@ -1970,6 +1995,8 @@ x86_emulate( > > switch ( b ) > { > + struct segment_register cs; > + > case 0x00 ... 0x05: add: /* add */ > emulate_2op_SrcV("add", src, dst, _regs.eflags); > break; > @@ -2031,7 +2058,7 @@ x86_emulate( > if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), > &dst.val, op_bytes, ctxt, ops)) != 0 ) > goto done; > - if ( (rc = load_seg(src.val, dst.val, 0, ctxt, ops)) != 0 ) > + if ( (rc = load_seg(src.val, dst.val, 0, NULL, ctxt, ops)) != 0 ) > return rc; > break; > > @@ -2364,7 +2391,7 @@ x86_emulate( > enum x86_segment seg = decode_segment(modrm_reg); > generate_exception_if(seg == decode_segment_failed, EXC_UD, -1); > generate_exception_if(seg == x86_seg_cs, EXC_UD, -1); > - if ( (rc = load_seg(seg, src.val, 0, ctxt, ops)) != 0 ) > + if ( (rc = load_seg(seg, src.val, 0, NULL, ctxt, ops)) != 0 ) > goto done; > if ( seg == x86_seg_ss ) > ctxt->retire.flags.mov_ss = 1; > @@ -2439,14 +2466,15 @@ x86_emulate( > sel = insn_fetch_type(uint16_t); > > if ( (rc = ops->read_segment(x86_seg_cs, ®, ctxt)) || > - (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), > + (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || > + (validate_far_branch(&cs, eip), > + rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), > ®.sel, op_bytes, ctxt)) || > (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), > - &_regs.eip, op_bytes, ctxt)) ) > + &_regs.eip, op_bytes, ctxt)) || > + (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) ) > goto done; > > - if ( (rc = load_seg(x86_seg_cs, sel, 0, ctxt, ops)) != 0 ) > - goto done; > _regs.eip = eip; > break; > } > @@ -2664,7 +2692,8 @@ x86_emulate( > int offset = (b == 0xc2) ? insn_fetch_type(uint16_t) : 0; > op_bytes = ((op_bytes == 4) && mode_64bit()) ? 8 : op_bytes; > if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes + offset), > - &dst.val, op_bytes, ctxt, ops)) != 0 ) > + &dst.val, op_bytes, ctxt, ops)) != 0 || > + (rc = ops->insn_fetch(x86_seg_cs, dst.val, NULL, 0, ctxt)) ) > goto done; > _regs.eip = dst.val; > break; > @@ -2679,7 +2708,7 @@ x86_emulate( > if ( (rc = read_ulong(src.mem.seg, src.mem.off + src.bytes, > &sel, 2, ctxt, ops)) != 0 ) > goto done; > - if ( (rc = load_seg(dst.val, sel, 0, ctxt, ops)) != 0 ) > + if ( (rc = load_seg(dst.val, sel, 0, NULL, ctxt, ops)) != 0 ) > goto done; > dst.val = src.val; > break; > @@ -2753,7 +2782,8 @@ x86_emulate( > &dst.val, op_bytes, ctxt, ops)) || > (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes + offset), > &src.val, op_bytes, ctxt, ops)) || > - (rc = load_seg(x86_seg_cs, src.val, 1, ctxt, ops)) ) > + (rc = load_seg(x86_seg_cs, src.val, 1, &cs, ctxt, ops)) || > + (rc = commit_far_branch(&cs, dst.val)) ) > goto done; > _regs.eip = dst.val; > break; > @@ -2782,7 +2812,7 @@ x86_emulate( > goto swint; > > case 0xcf: /* iret */ { > - unsigned long cs, eip, eflags; > + unsigned long sel, eip, eflags; > uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM; > if ( !mode_ring0() ) > mask |= EFLG_IOPL; > @@ -2792,7 +2822,7 @@ x86_emulate( > if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), > &eip, op_bytes, ctxt, ops)) || > (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), > - &cs, op_bytes, ctxt, ops)) || > + &sel, op_bytes, ctxt, ops)) || > (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), > &eflags, op_bytes, ctxt, ops)) ) > goto done; > @@ -2802,7 +2832,8 @@ x86_emulate( > _regs.eflags &= mask; > _regs.eflags |= (uint32_t)(eflags & ~mask) | 0x02; > _regs.eip = eip; > - if ( (rc = load_seg(x86_seg_cs, cs, 1, ctxt, ops)) != 0 ) > + if ( (rc = load_seg(x86_seg_cs, sel, 1, &cs, ctxt, ops)) || > + (rc = commit_far_branch(&cs, eip)) ) > goto done; > break; > } > @@ -3432,7 +3463,8 @@ x86_emulate( > generate_exception_if(mode_64bit(), EXC_UD, -1); > eip = insn_fetch_bytes(op_bytes); > sel = insn_fetch_type(uint16_t); > - if ( (rc = load_seg(x86_seg_cs, sel, 0, ctxt, ops)) != 0 ) > + if ( (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || > + (rc = commit_far_branch(&cs, eip)) ) > goto done; > _regs.eip = eip; > break; > @@ -3702,10 +3734,14 @@ x86_emulate( > break; > case 2: /* call (near) */ > dst.val = _regs.eip; > + if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) > ) > + goto done; > _regs.eip = src.val; > src.val = dst.val; > goto push; > case 4: /* jmp (near) */ > + if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) > ) > + goto done; > _regs.eip = src.val; > dst.type = OP_NONE; > break; > @@ -3724,14 +3760,17 @@ x86_emulate( > struct segment_register reg; > fail_if(ops->read_segment == NULL); > if ( (rc = ops->read_segment(x86_seg_cs, ®, ctxt)) || > - (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), > + (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || > + (validate_far_branch(&cs, src.val), > + rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), > ®.sel, op_bytes, ctxt)) || > (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), > - &_regs.eip, op_bytes, ctxt)) ) > + &_regs.eip, op_bytes, ctxt)) || > + (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) ) > goto done; > } > - > - if ( (rc = load_seg(x86_seg_cs, sel, 0, ctxt, ops)) != 0 ) > + else if ( (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || > + (rc = commit_far_branch(&cs, src.val)) ) > goto done; > _regs.eip = src.val; > > @@ -3816,7 +3855,7 @@ x86_emulate( > generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1); > generate_exception_if(!mode_ring0(), EXC_GP, 0); > if ( (rc = load_seg((modrm_reg & 1) ? x86_seg_tr : x86_seg_ldtr, > - src.val, 0, ctxt, ops)) != 0 ) > + src.val, 0, NULL, ctxt, ops)) != 0 ) > goto done; > break; > > @@ -4269,6 +4308,9 @@ x86_emulate( > goto done; > > generate_exception_if(!(msr_content & 0xfffc), EXC_GP, 0); > + generate_exception_if(user64 && (!is_canonical_address(_regs.edx) || > + !is_canonical_address(_regs.ecx)), > + EXC_GP, 0); > > cs.sel = (msr_content | 3) + /* SELECTOR_RPL_MASK */ > (user64 ? 32 : 16);
At 07:52 -0700 on 25 Feb (1456386721), Jan Beulich wrote: > >>> On 17.02.16 at 17:35, <JBeulich@suse.com> wrote: > > All branches need to #GP when their target violates the segment limit > > (in 16- and 32-bit modes) or is non-canonical (in 64-bit mode). For > > near branches facilitate this via a zero-byte instruction fetch from > > the target address (resulting in address translation and validation > > without an actual read from memory), while far branches get dealt with > > by breaking up the segment register loading into a read-and-validate > > part and a write one. The latter at once allows correcting some > > ordering issues in how the individual emulation steps get carried out: > > Before updating machine state, all exceptions unrelated to that state > > updating should have got raised (i.e. the only ones possibly resulting > > in partly updated state are faulting memory writes [pushes]). > > > > Note that while not immediately needed here, write and distinct read > > emulation routines get updated to deal with zero byte accesses too, for > > overall consistency. > > > > Reported-by: å??令 <liuling-it@360.cn> > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Sorry, hadn't spotted the shadow change. Acked-by: Tim Deegan <tim@xen.org>
--- a/tools/tests/x86_emulator/x86_emulate.c +++ b/tools/tests/x86_emulator/x86_emulate.c @@ -8,6 +8,8 @@ typedef bool bool_t; +#define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63)) + #define BUG() abort() #define ASSERT assert --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -745,7 +745,7 @@ static int __hvmemul_read( rc = hvmemul_virtual_to_linear( seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr); - if ( rc != X86EMUL_OKAY ) + if ( rc != X86EMUL_OKAY || !bytes ) return rc; if ( ((access_type != hvm_access_insn_fetch ? vio->mmio_access.read_access @@ -811,13 +811,17 @@ static int hvmemul_insn_fetch( container_of(ctxt, struct hvm_emulate_ctxt, ctxt); unsigned int insn_off = offset - hvmemul_ctxt->insn_buf_eip; - /* Fall back if requested bytes are not in the prefetch cache. */ - if ( unlikely((insn_off + bytes) > hvmemul_ctxt->insn_buf_bytes) ) + /* + * Fall back if requested bytes are not in the prefetch cache. + * But always perform the (fake) read when bytes == 0. + */ + if ( !bytes || + unlikely((insn_off + bytes) > hvmemul_ctxt->insn_buf_bytes) ) { int rc = __hvmemul_read(seg, offset, p_data, bytes, hvm_access_insn_fetch, hvmemul_ctxt); - if ( rc == X86EMUL_OKAY ) + if ( rc == X86EMUL_OKAY && bytes ) { ASSERT(insn_off + bytes <= sizeof(hvmemul_ctxt->insn_buf)); memcpy(&hvmemul_ctxt->insn_buf[insn_off], p_data, bytes); @@ -849,7 +853,7 @@ static int hvmemul_write( rc = hvmemul_virtual_to_linear( seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr); - if ( rc != X86EMUL_OKAY ) + if ( rc != X86EMUL_OKAY || !bytes ) return rc; if ( vio->mmio_access.write_access && --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3685,7 +3685,7 @@ int hvm_virtual_to_linear_addr( * Certain of them are not done in native real mode anyway. */ addr = (uint32_t)(addr + reg->base); - last_byte = (uint32_t)addr + bytes - 1; + last_byte = (uint32_t)addr + bytes - !!bytes; if ( last_byte < addr ) return 0; } @@ -3709,7 +3709,7 @@ int hvm_virtual_to_linear_addr( break; } - last_byte = (uint32_t)offset + bytes - 1; + last_byte = (uint32_t)offset + bytes - !!bytes; /* Is this a grows-down data segment? Special limit check if so. */ if ( (reg->attr.fields.type & 0xc) == 0x4 ) @@ -3740,7 +3740,7 @@ int hvm_virtual_to_linear_addr( if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) ) addr += reg->base; - last_byte = addr + bytes - 1; + last_byte = addr + bytes - !!bytes; if ( !is_canonical_address(addr) || last_byte < addr || !is_canonical_address(last_byte) ) return 0; --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -157,7 +157,7 @@ hvm_read(enum x86_segment seg, rc = hvm_translate_linear_addr( seg, offset, bytes, access_type, sh_ctxt, &addr); - if ( rc ) + if ( rc || !bytes ) return rc; if ( access_type == hvm_access_insn_fetch ) @@ -241,7 +241,7 @@ hvm_emulate_write(enum x86_segment seg, rc = hvm_translate_linear_addr( seg, offset, bytes, hvm_access_write, sh_ctxt, &addr); - if ( rc ) + if ( rc || !bytes ) return rc; return v->arch.paging.mode->shadow.x86_emulate_write( --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -5126,10 +5126,11 @@ static int ptwr_emulated_read( unsigned int bytes, struct x86_emulate_ctxt *ctxt) { - unsigned int rc; + unsigned int rc = bytes; unsigned long addr = offset; - if ( (rc = copy_from_user(p_data, (void *)addr, bytes)) != 0 ) + if ( !__addr_ok(addr) || + (rc = __copy_from_user(p_data, (void *)addr, bytes)) ) { propagate_page_fault(addr + bytes - rc, 0); /* read fault */ return X86EMUL_EXCEPTION; @@ -5278,7 +5279,7 @@ static int ptwr_emulated_write( { paddr_t val = 0; - if ( (bytes > sizeof(paddr_t)) || (bytes & (bytes -1)) ) + if ( (bytes > sizeof(paddr_t)) || (bytes & (bytes - 1)) || !bytes ) { MEM_LOG("ptwr_emulate: bad write size (addr=%lx, bytes=%u)", offset, bytes); @@ -5394,7 +5395,8 @@ int mmio_ro_emulated_write( struct mmio_ro_emulate_ctxt *mmio_ro_ctxt = ctxt->data; /* Only allow naturally-aligned stores at the original %cr2 address. */ - if ( ((bytes | offset) & (bytes - 1)) || offset != mmio_ro_ctxt->cr2 ) + if ( ((bytes | offset) & (bytes - 1)) || !bytes || + offset != mmio_ro_ctxt->cr2 ) { MEM_LOG("mmio_ro_emulate: bad access (cr2=%lx, addr=%lx, bytes=%u)", mmio_ro_ctxt->cr2, offset, bytes); @@ -5423,7 +5425,7 @@ int mmcfg_intercept_write( * Only allow naturally-aligned stores no wider than 4 bytes to the * original %cr2 address. */ - if ( ((bytes | offset) & (bytes - 1)) || bytes > 4 || + if ( ((bytes | offset) & (bytes - 1)) || bytes > 4 || !bytes || offset != mmio_ctxt->cr2 ) { MEM_LOG("mmcfg_intercept: bad write (cr2=%lx, addr=%lx, bytes=%u)", --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -642,14 +642,26 @@ do { #define jmp_rel(rel) \ do { \ - int _rel = (int)(rel); \ - _regs.eip += _rel; \ + unsigned long ip = _regs.eip + (int)(rel); \ if ( op_bytes == 2 ) \ - _regs.eip = (uint16_t)_regs.eip; \ + ip = (uint16_t)ip; \ else if ( !mode_64bit() ) \ - _regs.eip = (uint32_t)_regs.eip; \ + ip = (uint32_t)ip; \ + rc = ops->insn_fetch(x86_seg_cs, ip, NULL, 0, ctxt); \ + if ( rc ) goto done; \ + _regs.eip = ip; \ } while (0) +#define validate_far_branch(cs, ip) \ + generate_exception_if(in_longmode(ctxt, ops) && (cs)->attr.fields.l \ + ? !is_canonical_address(ip) \ + : (ip) > (cs)->limit, EXC_GP, 0) + +#define commit_far_branch(cs, ip) ({ \ + validate_far_branch(cs, ip); \ + ops->write_segment(x86_seg_cs, cs, ctxt); \ +}) + struct fpu_insn_ctxt { uint8_t insn_bytes; uint8_t exn_raised; @@ -1099,29 +1111,30 @@ static int realmode_load_seg( enum x86_segment seg, uint16_t sel, + struct segment_register *sreg, struct x86_emulate_ctxt *ctxt, const struct x86_emulate_ops *ops) { - struct segment_register reg; - int rc; + int rc = ops->read_segment(seg, sreg, ctxt); - if ( (rc = ops->read_segment(seg, ®, ctxt)) != 0 ) - return rc; - - reg.sel = sel; - reg.base = (uint32_t)sel << 4; + if ( !rc ) + { + sreg->sel = sel; + sreg->base = (uint32_t)sel << 4; + } - return ops->write_segment(seg, ®, ctxt); + return rc; } static int protmode_load_seg( enum x86_segment seg, uint16_t sel, bool_t is_ret, + struct segment_register *sreg, struct x86_emulate_ctxt *ctxt, const struct x86_emulate_ops *ops) { - struct segment_register desctab, ss, segr; + struct segment_register desctab, ss; struct { uint32_t a, b; } desc; uint8_t dpl, rpl, cpl; uint32_t new_desc_b, a_flag = 0x100; @@ -1132,8 +1145,8 @@ protmode_load_seg( { if ( (seg == x86_seg_cs) || (seg == x86_seg_ss) ) goto raise_exn; - memset(&segr, 0, sizeof(segr)); - return ops->write_segment(seg, &segr, ctxt); + memset(sreg, 0, sizeof(*sreg)); + return X86EMUL_OKAY; } /* System segment descriptors must reside in the GDT. */ @@ -1242,16 +1255,16 @@ protmode_load_seg( desc.b |= a_flag; skip_accessed_flag: - segr.base = (((desc.b << 0) & 0xff000000u) | - ((desc.b << 16) & 0x00ff0000u) | - ((desc.a >> 16) & 0x0000ffffu)); - segr.attr.bytes = (((desc.b >> 8) & 0x00ffu) | - ((desc.b >> 12) & 0x0f00u)); - segr.limit = (desc.b & 0x000f0000u) | (desc.a & 0x0000ffffu); - if ( segr.attr.fields.g ) - segr.limit = (segr.limit << 12) | 0xfffu; - segr.sel = sel; - return ops->write_segment(seg, &segr, ctxt); + sreg->base = (((desc.b << 0) & 0xff000000u) | + ((desc.b << 16) & 0x00ff0000u) | + ((desc.a >> 16) & 0x0000ffffu)); + sreg->attr.bytes = (((desc.b >> 8) & 0x00ffu) | + ((desc.b >> 12) & 0x0f00u)); + sreg->limit = (desc.b & 0x000f0000u) | (desc.a & 0x0000ffffu); + if ( sreg->attr.fields.g ) + sreg->limit = (sreg->limit << 12) | 0xfffu; + sreg->sel = sel; + return X86EMUL_OKAY; raise_exn: if ( ops->inject_hw_exception == NULL ) @@ -1265,17 +1278,29 @@ static int load_seg( enum x86_segment seg, uint16_t sel, bool_t is_ret, + struct segment_register *sreg, struct x86_emulate_ctxt *ctxt, const struct x86_emulate_ops *ops) { + struct segment_register reg; + int rc; + if ( (ops->read_segment == NULL) || (ops->write_segment == NULL) ) return X86EMUL_UNHANDLEABLE; + if ( !sreg ) + sreg = ® + if ( in_protmode(ctxt, ops) ) - return protmode_load_seg(seg, sel, is_ret, ctxt, ops); + rc = protmode_load_seg(seg, sel, is_ret, sreg, ctxt, ops); + else + rc = realmode_load_seg(seg, sel, sreg, ctxt, ops); + + if ( !rc && sreg == ® ) + rc = ops->write_segment(seg, sreg, ctxt); - return realmode_load_seg(seg, sel, ctxt, ops); + return rc; } void * @@ -1970,6 +1995,8 @@ x86_emulate( switch ( b ) { + struct segment_register cs; + case 0x00 ... 0x05: add: /* add */ emulate_2op_SrcV("add", src, dst, _regs.eflags); break; @@ -2031,7 +2058,7 @@ x86_emulate( if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), &dst.val, op_bytes, ctxt, ops)) != 0 ) goto done; - if ( (rc = load_seg(src.val, dst.val, 0, ctxt, ops)) != 0 ) + if ( (rc = load_seg(src.val, dst.val, 0, NULL, ctxt, ops)) != 0 ) return rc; break; @@ -2364,7 +2391,7 @@ x86_emulate( enum x86_segment seg = decode_segment(modrm_reg); generate_exception_if(seg == decode_segment_failed, EXC_UD, -1); generate_exception_if(seg == x86_seg_cs, EXC_UD, -1); - if ( (rc = load_seg(seg, src.val, 0, ctxt, ops)) != 0 ) + if ( (rc = load_seg(seg, src.val, 0, NULL, ctxt, ops)) != 0 ) goto done; if ( seg == x86_seg_ss ) ctxt->retire.flags.mov_ss = 1; @@ -2439,14 +2466,15 @@ x86_emulate( sel = insn_fetch_type(uint16_t); if ( (rc = ops->read_segment(x86_seg_cs, ®, ctxt)) || - (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), + (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || + (validate_far_branch(&cs, eip), + rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), ®.sel, op_bytes, ctxt)) || (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), - &_regs.eip, op_bytes, ctxt)) ) + &_regs.eip, op_bytes, ctxt)) || + (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) ) goto done; - if ( (rc = load_seg(x86_seg_cs, sel, 0, ctxt, ops)) != 0 ) - goto done; _regs.eip = eip; break; } @@ -2664,7 +2692,8 @@ x86_emulate( int offset = (b == 0xc2) ? insn_fetch_type(uint16_t) : 0; op_bytes = ((op_bytes == 4) && mode_64bit()) ? 8 : op_bytes; if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes + offset), - &dst.val, op_bytes, ctxt, ops)) != 0 ) + &dst.val, op_bytes, ctxt, ops)) != 0 || + (rc = ops->insn_fetch(x86_seg_cs, dst.val, NULL, 0, ctxt)) ) goto done; _regs.eip = dst.val; break; @@ -2679,7 +2708,7 @@ x86_emulate( if ( (rc = read_ulong(src.mem.seg, src.mem.off + src.bytes, &sel, 2, ctxt, ops)) != 0 ) goto done; - if ( (rc = load_seg(dst.val, sel, 0, ctxt, ops)) != 0 ) + if ( (rc = load_seg(dst.val, sel, 0, NULL, ctxt, ops)) != 0 ) goto done; dst.val = src.val; break; @@ -2753,7 +2782,8 @@ x86_emulate( &dst.val, op_bytes, ctxt, ops)) || (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes + offset), &src.val, op_bytes, ctxt, ops)) || - (rc = load_seg(x86_seg_cs, src.val, 1, ctxt, ops)) ) + (rc = load_seg(x86_seg_cs, src.val, 1, &cs, ctxt, ops)) || + (rc = commit_far_branch(&cs, dst.val)) ) goto done; _regs.eip = dst.val; break; @@ -2782,7 +2812,7 @@ x86_emulate( goto swint; case 0xcf: /* iret */ { - unsigned long cs, eip, eflags; + unsigned long sel, eip, eflags; uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM; if ( !mode_ring0() ) mask |= EFLG_IOPL; @@ -2792,7 +2822,7 @@ x86_emulate( if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), &eip, op_bytes, ctxt, ops)) || (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), - &cs, op_bytes, ctxt, ops)) || + &sel, op_bytes, ctxt, ops)) || (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes), &eflags, op_bytes, ctxt, ops)) ) goto done; @@ -2802,7 +2832,8 @@ x86_emulate( _regs.eflags &= mask; _regs.eflags |= (uint32_t)(eflags & ~mask) | 0x02; _regs.eip = eip; - if ( (rc = load_seg(x86_seg_cs, cs, 1, ctxt, ops)) != 0 ) + if ( (rc = load_seg(x86_seg_cs, sel, 1, &cs, ctxt, ops)) || + (rc = commit_far_branch(&cs, eip)) ) goto done; break; } @@ -3432,7 +3463,8 @@ x86_emulate( generate_exception_if(mode_64bit(), EXC_UD, -1); eip = insn_fetch_bytes(op_bytes); sel = insn_fetch_type(uint16_t); - if ( (rc = load_seg(x86_seg_cs, sel, 0, ctxt, ops)) != 0 ) + if ( (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || + (rc = commit_far_branch(&cs, eip)) ) goto done; _regs.eip = eip; break; @@ -3702,10 +3734,14 @@ x86_emulate( break; case 2: /* call (near) */ dst.val = _regs.eip; + if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) ) + goto done; _regs.eip = src.val; src.val = dst.val; goto push; case 4: /* jmp (near) */ + if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) ) + goto done; _regs.eip = src.val; dst.type = OP_NONE; break; @@ -3724,14 +3760,17 @@ x86_emulate( struct segment_register reg; fail_if(ops->read_segment == NULL); if ( (rc = ops->read_segment(x86_seg_cs, ®, ctxt)) || - (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), + (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || + (validate_far_branch(&cs, src.val), + rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), ®.sel, op_bytes, ctxt)) || (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes), - &_regs.eip, op_bytes, ctxt)) ) + &_regs.eip, op_bytes, ctxt)) || + (rc = ops->write_segment(x86_seg_cs, &cs, ctxt)) ) goto done; } - - if ( (rc = load_seg(x86_seg_cs, sel, 0, ctxt, ops)) != 0 ) + else if ( (rc = load_seg(x86_seg_cs, sel, 0, &cs, ctxt, ops)) || + (rc = commit_far_branch(&cs, src.val)) ) goto done; _regs.eip = src.val; @@ -3816,7 +3855,7 @@ x86_emulate( generate_exception_if(!in_protmode(ctxt, ops), EXC_UD, -1); generate_exception_if(!mode_ring0(), EXC_GP, 0); if ( (rc = load_seg((modrm_reg & 1) ? x86_seg_tr : x86_seg_ldtr, - src.val, 0, ctxt, ops)) != 0 ) + src.val, 0, NULL, ctxt, ops)) != 0 ) goto done; break; @@ -4269,6 +4308,9 @@ x86_emulate( goto done; generate_exception_if(!(msr_content & 0xfffc), EXC_GP, 0); + generate_exception_if(user64 && (!is_canonical_address(_regs.edx) || + !is_canonical_address(_regs.ecx)), + EXC_GP, 0); cs.sel = (msr_content | 3) + /* SELECTOR_RPL_MASK */ (user64 ? 32 : 16);