* [PATCH 0/5] x86: XSA-170 follow-up
@ 2016-02-17 16:21 Jan Beulich
2016-02-17 16:32 ` [PATCH 1/5] x86emul: fix rIP handling Jan Beulich
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Jan Beulich @ 2016-02-17 16:21 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser
Patch 1 was sent out before (as replacement proposal to another
patch), and is unchanged (still awaiting feedback, but fitting pretty
nicely with the topic of XSA-170).
Patch 2 fixes the actual problem which was reported to us,
resulting in XSA-170.
The remaining patches are only loosely related, but were
found desirable / put together in the course of investigating
the security issue.
1: x86emul: fix rIP handling
2: x86emul: limit-check branch targets
3: x86emul: simplify IRET logic
4: VMX: fold redundant code
5: x86: drop failsafe callback invocation from assembly
Signed-off-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/5] x86emul: fix rIP handling
2016-02-17 16:21 [PATCH 0/5] x86: XSA-170 follow-up Jan Beulich
@ 2016-02-17 16:32 ` Jan Beulich
2016-02-17 17:51 ` Andrew Cooper
2016-02-17 16:35 ` [PATCH 2/5] x86emul: limit-check branch targets Jan Beulich
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2016-02-17 16:32 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]
Deal with rIP just like with any other register: Truncate to designated
width upon entry, write back the zero-extended 32-bit value when
emulating 32-bit code, and leave the upper 48 bits unchanged for 16-bit
code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -570,7 +570,6 @@ do{ asm volatile (
/* Fetch next part of the instruction being emulated. */
#define insn_fetch_bytes(_size) \
({ unsigned long _x = 0, _eip = _regs.eip; \
- if ( !mode_64bit() ) _eip = (uint32_t)_eip; /* ignore upper dword */ \
_regs.eip += (_size); /* real hardware doesn't truncate */ \
generate_exception_if((uint8_t)(_regs.eip - \
ctxt->regs->eip) > MAX_INST_LEN, \
@@ -1492,6 +1491,10 @@ x86_emulate(
#endif
}
+ /* Truncate rIP to def_ad_bytes (2 or 4) if necessary. */
+ if ( def_ad_bytes < sizeof(_regs.eip) )
+ _regs.eip &= (1UL << (def_ad_bytes * 8)) - 1;
+
/* Prefix bytes. */
for ( ; ; )
{
@@ -3783,6 +3786,21 @@ x86_emulate(
/* Commit shadow register state. */
_regs.eflags &= ~EFLG_RF;
+ switch ( __builtin_expect(def_ad_bytes, sizeof(_regs.eip)) )
+ {
+ uint16_t ip;
+
+ case 2:
+ ip = _regs.eip;
+ _regs.eip = ctxt->regs->eip;
+ *(uint16_t *)&_regs.eip = ip;
+ break;
+#ifdef __x86_64__
+ case 4:
+ _regs.rip = _regs._eip;
+ break;
+#endif
+ }
*ctxt->regs = _regs;
done:
[-- Attachment #2: x86emul-truncate-IP.patch --]
[-- Type: text/plain, Size: 1717 bytes --]
x86emul: fix rIP handling
Deal with rIP just like with any other register: Truncate to designated
width upon entry, write back the zero-extended 32-bit value when
emulating 32-bit code, and leave the upper 48 bits unchanged for 16-bit
code.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -570,7 +570,6 @@ do{ asm volatile (
/* Fetch next part of the instruction being emulated. */
#define insn_fetch_bytes(_size) \
({ unsigned long _x = 0, _eip = _regs.eip; \
- if ( !mode_64bit() ) _eip = (uint32_t)_eip; /* ignore upper dword */ \
_regs.eip += (_size); /* real hardware doesn't truncate */ \
generate_exception_if((uint8_t)(_regs.eip - \
ctxt->regs->eip) > MAX_INST_LEN, \
@@ -1492,6 +1491,10 @@ x86_emulate(
#endif
}
+ /* Truncate rIP to def_ad_bytes (2 or 4) if necessary. */
+ if ( def_ad_bytes < sizeof(_regs.eip) )
+ _regs.eip &= (1UL << (def_ad_bytes * 8)) - 1;
+
/* Prefix bytes. */
for ( ; ; )
{
@@ -3783,6 +3786,21 @@ x86_emulate(
/* Commit shadow register state. */
_regs.eflags &= ~EFLG_RF;
+ switch ( __builtin_expect(def_ad_bytes, sizeof(_regs.eip)) )
+ {
+ uint16_t ip;
+
+ case 2:
+ ip = _regs.eip;
+ _regs.eip = ctxt->regs->eip;
+ *(uint16_t *)&_regs.eip = ip;
+ break;
+#ifdef __x86_64__
+ case 4:
+ _regs.rip = _regs._eip;
+ break;
+#endif
+ }
*ctxt->regs = _regs;
done:
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] x86emul: limit-check branch targets
2016-02-17 16:21 [PATCH 0/5] x86: XSA-170 follow-up Jan Beulich
2016-02-17 16:32 ` [PATCH 1/5] x86emul: fix rIP handling Jan Beulich
@ 2016-02-17 16:35 ` Jan Beulich
2016-02-17 17:59 ` Andrew Cooper
2016-02-25 14:52 ` Ping: " Jan Beulich
2016-02-17 16:36 ` [PATCH 3/5] x86emul: simplify IRET logic Jan Beulich
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Jan Beulich @ 2016-02-17 16:35 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Keir Fraser, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 19496 bytes --]
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);
[-- Attachment #2: x86emul-branch-limit-checks.patch --]
[-- Type: text/plain, Size: 19563 bytes --]
x86emul: limit-check branch targets
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);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/5] x86emul: simplify IRET logic
2016-02-17 16:21 [PATCH 0/5] x86: XSA-170 follow-up Jan Beulich
2016-02-17 16:32 ` [PATCH 1/5] x86emul: fix rIP handling Jan Beulich
2016-02-17 16:35 ` [PATCH 2/5] x86emul: limit-check branch targets Jan Beulich
@ 2016-02-17 16:36 ` Jan Beulich
2016-02-17 18:03 ` Andrew Cooper
2016-02-17 16:37 ` [PATCH 4/5] VMX: fold redundant code Jan Beulich
2016-02-17 16:38 ` [PATCH 5/5] x86: drop failsafe callback invocation from assembly Jan Beulich
4 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2016-02-17 16:36 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]
Since we only handle real mode, we need to consider neither non-ring0
nor IOPL. Also for POPF the mode_iopl() check can really be inside the
not-ring-0 body.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2490,9 +2490,11 @@ x86_emulate(
case 0x9d: /* popf */ {
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
if ( !mode_ring0() )
+ {
mask |= EFLG_IOPL;
- if ( !mode_iopl() )
- mask |= EFLG_IF;
+ if ( !mode_iopl() )
+ mask |= EFLG_IF;
+ }
/* 64-bit mode: POP defaults to a 64-bit operand. */
if ( mode_64bit() && (op_bytes == 4) )
op_bytes = 8;
@@ -2814,10 +2816,7 @@ x86_emulate(
case 0xcf: /* iret */ {
unsigned long sel, eip, eflags;
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
- if ( !mode_ring0() )
- mask |= EFLG_IOPL;
- if ( !mode_iopl() )
- mask |= EFLG_IF;
+
fail_if(!in_realmode(ctxt, ops));
if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes),
&eip, op_bytes, ctxt, ops)) ||
@@ -2830,7 +2829,7 @@ x86_emulate(
eflags = (uint16_t)eflags | (_regs.eflags & 0xffff0000u);
eflags &= 0x257fd5;
_regs.eflags &= mask;
- _regs.eflags |= (uint32_t)(eflags & ~mask) | 0x02;
+ _regs.eflags |= (eflags & ~mask) | 0x02;
_regs.eip = eip;
if ( (rc = load_seg(x86_seg_cs, sel, 1, &cs, ctxt, ops)) ||
(rc = commit_far_branch(&cs, eip)) )
[-- Attachment #2: x86emul-iret-simplify.patch --]
[-- Type: text/plain, Size: 1718 bytes --]
x86emul: simplify IRET logic
Since we only handle real mode, we need to consider neither non-ring0
nor IOPL. Also for POPF the mode_iopl() check can really be inside the
not-ring-0 body.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2490,9 +2490,11 @@ x86_emulate(
case 0x9d: /* popf */ {
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
if ( !mode_ring0() )
+ {
mask |= EFLG_IOPL;
- if ( !mode_iopl() )
- mask |= EFLG_IF;
+ if ( !mode_iopl() )
+ mask |= EFLG_IF;
+ }
/* 64-bit mode: POP defaults to a 64-bit operand. */
if ( mode_64bit() && (op_bytes == 4) )
op_bytes = 8;
@@ -2814,10 +2816,7 @@ x86_emulate(
case 0xcf: /* iret */ {
unsigned long sel, eip, eflags;
uint32_t mask = EFLG_VIP | EFLG_VIF | EFLG_VM;
- if ( !mode_ring0() )
- mask |= EFLG_IOPL;
- if ( !mode_iopl() )
- mask |= EFLG_IF;
+
fail_if(!in_realmode(ctxt, ops));
if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes),
&eip, op_bytes, ctxt, ops)) ||
@@ -2830,7 +2829,7 @@ x86_emulate(
eflags = (uint16_t)eflags | (_regs.eflags & 0xffff0000u);
eflags &= 0x257fd5;
_regs.eflags &= mask;
- _regs.eflags |= (uint32_t)(eflags & ~mask) | 0x02;
+ _regs.eflags |= (eflags & ~mask) | 0x02;
_regs.eip = eip;
if ( (rc = load_seg(x86_seg_cs, sel, 1, &cs, ctxt, ops)) ||
(rc = commit_far_branch(&cs, eip)) )
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/5] VMX: fold redundant code
2016-02-17 16:21 [PATCH 0/5] x86: XSA-170 follow-up Jan Beulich
` (2 preceding siblings ...)
2016-02-17 16:36 ` [PATCH 3/5] x86emul: simplify IRET logic Jan Beulich
@ 2016-02-17 16:37 ` Jan Beulich
2016-02-17 18:05 ` Andrew Cooper
2016-02-18 5:26 ` Tian, Kevin
2016-02-17 16:38 ` [PATCH 5/5] x86: drop failsafe callback invocation from assembly Jan Beulich
4 siblings, 2 replies; 14+ messages in thread
From: Jan Beulich @ 2016-02-17 16:37 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 938 bytes --]
No need to do this in two slightly different ways, possibly keeping the
compiler from folding the code for us.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3103,6 +3103,7 @@ void vmx_vmexit_handler(struct cpu_user_
&& vector != TRAP_nmi
&& vector != TRAP_machine_check )
{
+ default:
perfc_incr(realmode_exits);
v->arch.hvm_vmx.vmx_emulate = 1;
HVMTRACE_0D(REALMODE_EMULATE);
@@ -3121,12 +3122,6 @@ void vmx_vmexit_handler(struct cpu_user_
case EXIT_REASON_INVEPT:
case EXIT_REASON_INVVPID:
break;
-
- default:
- v->arch.hvm_vmx.vmx_emulate = 1;
- perfc_incr(realmode_exits);
- HVMTRACE_0D(REALMODE_EMULATE);
- return;
}
}
[-- Attachment #2: VMX-fold-realmode-exit-code.patch --]
[-- Type: text/plain, Size: 960 bytes --]
VMX: fold redundant code
No need to do this in two slightly different ways, possibly keeping the
compiler from folding the code for us.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3103,6 +3103,7 @@ void vmx_vmexit_handler(struct cpu_user_
&& vector != TRAP_nmi
&& vector != TRAP_machine_check )
{
+ default:
perfc_incr(realmode_exits);
v->arch.hvm_vmx.vmx_emulate = 1;
HVMTRACE_0D(REALMODE_EMULATE);
@@ -3121,12 +3122,6 @@ void vmx_vmexit_handler(struct cpu_user_
case EXIT_REASON_INVEPT:
case EXIT_REASON_INVVPID:
break;
-
- default:
- v->arch.hvm_vmx.vmx_emulate = 1;
- perfc_incr(realmode_exits);
- HVMTRACE_0D(REALMODE_EMULATE);
- return;
}
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] x86: drop failsafe callback invocation from assembly
2016-02-17 16:21 [PATCH 0/5] x86: XSA-170 follow-up Jan Beulich
` (3 preceding siblings ...)
2016-02-17 16:37 ` [PATCH 4/5] VMX: fold redundant code Jan Beulich
@ 2016-02-17 16:38 ` Jan Beulich
2016-02-17 19:01 ` Andrew Cooper
4 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2016-02-17 16:38 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 6447 bytes --]
Afaict this was never necessary on a 64-bit hypervisor, and was instead
just blindly cloned over from 32-bit code: We don't fiddle with (and
hence don't reload) any of DS, ES, FS, or GS, and an exception on IRET
itself can equally well be reported to the guest as that very exception
on the target of that IRET.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -176,39 +176,7 @@ ENTRY(compat_restore_all_guest)
ASSERT_INTERRUPTS_DISABLED
RESTORE_ALL adj=8 compat=1
.Lft0: iretq
-
-.section .fixup,"ax"
-.Lfx0: sti
- SAVE_ALL
- movq UREGS_error_code(%rsp),%rsi
- movq %rsp,%rax
- andq $~0xf,%rsp
- pushq $__HYPERVISOR_DS # SS
- pushq %rax # RSP
- pushfq # RFLAGS
- pushq $__HYPERVISOR_CS # CS
- leaq .Ldf0(%rip),%rax
- pushq %rax # RIP
- pushq %rsi # error_code/entry_vector
- jmp handle_exception
-.Ldf0: GET_CURRENT(%rbx)
- jmp compat_test_all_events
-compat_failsafe_callback:
- GET_CURRENT(%rbx)
- leaq VCPU_trap_bounce(%rbx),%rdx
- movl VCPU_failsafe_addr(%rbx),%eax
- movl %eax,TRAPBOUNCE_eip(%rdx)
- movl VCPU_failsafe_sel(%rbx),%eax
- movw %ax,TRAPBOUNCE_cs(%rdx)
- movb $TBF_FAILSAFE,TRAPBOUNCE_flags(%rdx)
- btq $_VGCF_failsafe_disables_events,VCPU_guest_context_flags(%rbx)
- jnc 1f
- orb $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-1: call compat_create_bounce_frame
- jmp compat_test_all_events
-.previous
- _ASM_PRE_EXTABLE(.Lft0, .Lfx0)
- _ASM_EXTABLE(.Ldf0, compat_failsafe_callback)
+ _ASM_PRE_EXTABLE(.Lft0, handle_exception)
/* %rdx: trap_bounce, %rbx: struct vcpu */
ENTRY(compat_post_handle_exception)
@@ -322,17 +290,6 @@ compat_create_bounce_frame:
movl TRAPBOUNCE_error_code(%rdx),%eax
.Lft8: movl %eax,%fs:(%rsi) # ERROR CODE
1:
- testb $TBF_FAILSAFE,%cl
-UNLIKELY_START(nz, compat_bounce_failsafe)
- subl $4*4,%esi
- movl %gs,%eax
-.Lft9: movl %eax,%fs:3*4(%rsi) # GS
-.Lft10: movl %edi,%fs:2*4(%rsi) # FS
- movl %es,%eax
-.Lft11: movl %eax,%fs:1*4(%rsi) # ES
- movl %ds,%eax
-.Lft12: movl %eax,%fs:0*4(%rsi) # DS
-UNLIKELY_END(compat_bounce_failsafe)
/* Rewrite our stack frame and return to guest-OS mode. */
/* IA32 Ref. Vol. 3: TF, VM, RF and NT flags are cleared on trap. */
andl $~(X86_EFLAGS_VM|X86_EFLAGS_RF|\
@@ -364,14 +321,8 @@ __UNLIKELY_END(compat_bounce_null_select
_ASM_EXTABLE(.Lft6, compat_crash_page_fault_8)
_ASM_EXTABLE(.Lft7, compat_crash_page_fault)
_ASM_EXTABLE(.Lft8, compat_crash_page_fault)
- _ASM_EXTABLE(.Lft9, compat_crash_page_fault_12)
- _ASM_EXTABLE(.Lft10, compat_crash_page_fault_8)
- _ASM_EXTABLE(.Lft11, compat_crash_page_fault_4)
- _ASM_EXTABLE(.Lft12, compat_crash_page_fault)
_ASM_EXTABLE(.Lft13, .Lfx13)
-compat_crash_page_fault_12:
- addl $4,%esi
compat_crash_page_fault_8:
addl $4,%esi
compat_crash_page_fault_4:
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -64,37 +64,7 @@ restore_all_guest:
iret_exit_to_guest:
addq $8,%rsp
.Lft0: iretq
-
-.section .fixup,"ax"
-.Lfx0: sti
- SAVE_ALL
- movq UREGS_error_code(%rsp),%rsi
- movq %rsp,%rax
- andq $~0xf,%rsp
- pushq $__HYPERVISOR_DS # SS
- pushq %rax # RSP
- pushfq # RFLAGS
- pushq $__HYPERVISOR_CS # CS
- leaq .Ldf0(%rip),%rax
- pushq %rax # RIP
- pushq %rsi # error_code/entry_vector
- jmp handle_exception
-.Ldf0: GET_CURRENT(%rbx)
- jmp test_all_events
-failsafe_callback:
- GET_CURRENT(%rbx)
- leaq VCPU_trap_bounce(%rbx),%rdx
- movq VCPU_failsafe_addr(%rbx),%rax
- movq %rax,TRAPBOUNCE_eip(%rdx)
- movb $TBF_FAILSAFE,TRAPBOUNCE_flags(%rdx)
- bt $_VGCF_failsafe_disables_events,VCPU_guest_context_flags(%rbx)
- jnc 1f
- orb $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-1: call create_bounce_frame
- jmp test_all_events
-.previous
- _ASM_PRE_EXTABLE(.Lft0, .Lfx0)
- _ASM_EXTABLE(.Ldf0, failsafe_callback)
+ _ASM_PRE_EXTABLE(.Lft0, handle_exception)
ALIGN
/* No special register assumptions. */
@@ -405,18 +375,7 @@ __UNLIKELY_END(create_bounce_frame_bad_s
subq $8,%rsi
movl TRAPBOUNCE_error_code(%rdx),%eax
.Lft7: movq %rax,(%rsi) # ERROR CODE
-1: testb $TBF_FAILSAFE,%cl
-UNLIKELY_START(nz, bounce_failsafe)
- subq $32,%rsi
- movl %gs,%eax
-.Lft8: movq %rax,24(%rsi) # GS
- movl %fs,%eax
-.Lft9: movq %rax,16(%rsi) # FS
- movl %es,%eax
-.Lft10: movq %rax,8(%rsi) # ES
- movl %ds,%eax
-.Lft11: movq %rax,(%rsi) # DS
-UNLIKELY_END(bounce_failsafe)
+1:
subq $16,%rsi
movq UREGS_r11+8(%rsp),%rax
.Lft12: movq %rax,8(%rsi) # R11
@@ -446,10 +405,6 @@ __UNLIKELY_END(create_bounce_frame_bad_b
_ASM_EXTABLE(.Lft5, domain_crash_page_fault_16)
_ASM_EXTABLE(.Lft6, domain_crash_page_fault)
_ASM_EXTABLE(.Lft7, domain_crash_page_fault)
- _ASM_EXTABLE(.Lft8, domain_crash_page_fault_24)
- _ASM_EXTABLE(.Lft9, domain_crash_page_fault_16)
- _ASM_EXTABLE(.Lft10, domain_crash_page_fault_8)
- _ASM_EXTABLE(.Lft11, domain_crash_page_fault)
_ASM_EXTABLE(.Lft12, domain_crash_page_fault_8)
_ASM_EXTABLE(.Lft13, domain_crash_page_fault)
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -128,7 +128,6 @@
#define TBF_EXCEPTION 1
#define TBF_EXCEPTION_ERRCODE 2
#define TBF_INTERRUPT 8
-#define TBF_FAILSAFE 16
/* 'arch_vcpu' flags values */
#define _TF_kernel_mode 0
[-- Attachment #2: x86-no-failsafe-cb.patch --]
[-- Type: text/plain, Size: 6499 bytes --]
x86: drop failsafe callback invocation from assembly
Afaict this was never necessary on a 64-bit hypervisor, and was instead
just blindly cloned over from 32-bit code: We don't fiddle with (and
hence don't reload) any of DS, ES, FS, or GS, and an exception on IRET
itself can equally well be reported to the guest as that very exception
on the target of that IRET.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -176,39 +176,7 @@ ENTRY(compat_restore_all_guest)
ASSERT_INTERRUPTS_DISABLED
RESTORE_ALL adj=8 compat=1
.Lft0: iretq
-
-.section .fixup,"ax"
-.Lfx0: sti
- SAVE_ALL
- movq UREGS_error_code(%rsp),%rsi
- movq %rsp,%rax
- andq $~0xf,%rsp
- pushq $__HYPERVISOR_DS # SS
- pushq %rax # RSP
- pushfq # RFLAGS
- pushq $__HYPERVISOR_CS # CS
- leaq .Ldf0(%rip),%rax
- pushq %rax # RIP
- pushq %rsi # error_code/entry_vector
- jmp handle_exception
-.Ldf0: GET_CURRENT(%rbx)
- jmp compat_test_all_events
-compat_failsafe_callback:
- GET_CURRENT(%rbx)
- leaq VCPU_trap_bounce(%rbx),%rdx
- movl VCPU_failsafe_addr(%rbx),%eax
- movl %eax,TRAPBOUNCE_eip(%rdx)
- movl VCPU_failsafe_sel(%rbx),%eax
- movw %ax,TRAPBOUNCE_cs(%rdx)
- movb $TBF_FAILSAFE,TRAPBOUNCE_flags(%rdx)
- btq $_VGCF_failsafe_disables_events,VCPU_guest_context_flags(%rbx)
- jnc 1f
- orb $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-1: call compat_create_bounce_frame
- jmp compat_test_all_events
-.previous
- _ASM_PRE_EXTABLE(.Lft0, .Lfx0)
- _ASM_EXTABLE(.Ldf0, compat_failsafe_callback)
+ _ASM_PRE_EXTABLE(.Lft0, handle_exception)
/* %rdx: trap_bounce, %rbx: struct vcpu */
ENTRY(compat_post_handle_exception)
@@ -322,17 +290,6 @@ compat_create_bounce_frame:
movl TRAPBOUNCE_error_code(%rdx),%eax
.Lft8: movl %eax,%fs:(%rsi) # ERROR CODE
1:
- testb $TBF_FAILSAFE,%cl
-UNLIKELY_START(nz, compat_bounce_failsafe)
- subl $4*4,%esi
- movl %gs,%eax
-.Lft9: movl %eax,%fs:3*4(%rsi) # GS
-.Lft10: movl %edi,%fs:2*4(%rsi) # FS
- movl %es,%eax
-.Lft11: movl %eax,%fs:1*4(%rsi) # ES
- movl %ds,%eax
-.Lft12: movl %eax,%fs:0*4(%rsi) # DS
-UNLIKELY_END(compat_bounce_failsafe)
/* Rewrite our stack frame and return to guest-OS mode. */
/* IA32 Ref. Vol. 3: TF, VM, RF and NT flags are cleared on trap. */
andl $~(X86_EFLAGS_VM|X86_EFLAGS_RF|\
@@ -364,14 +321,8 @@ __UNLIKELY_END(compat_bounce_null_select
_ASM_EXTABLE(.Lft6, compat_crash_page_fault_8)
_ASM_EXTABLE(.Lft7, compat_crash_page_fault)
_ASM_EXTABLE(.Lft8, compat_crash_page_fault)
- _ASM_EXTABLE(.Lft9, compat_crash_page_fault_12)
- _ASM_EXTABLE(.Lft10, compat_crash_page_fault_8)
- _ASM_EXTABLE(.Lft11, compat_crash_page_fault_4)
- _ASM_EXTABLE(.Lft12, compat_crash_page_fault)
_ASM_EXTABLE(.Lft13, .Lfx13)
-compat_crash_page_fault_12:
- addl $4,%esi
compat_crash_page_fault_8:
addl $4,%esi
compat_crash_page_fault_4:
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -64,37 +64,7 @@ restore_all_guest:
iret_exit_to_guest:
addq $8,%rsp
.Lft0: iretq
-
-.section .fixup,"ax"
-.Lfx0: sti
- SAVE_ALL
- movq UREGS_error_code(%rsp),%rsi
- movq %rsp,%rax
- andq $~0xf,%rsp
- pushq $__HYPERVISOR_DS # SS
- pushq %rax # RSP
- pushfq # RFLAGS
- pushq $__HYPERVISOR_CS # CS
- leaq .Ldf0(%rip),%rax
- pushq %rax # RIP
- pushq %rsi # error_code/entry_vector
- jmp handle_exception
-.Ldf0: GET_CURRENT(%rbx)
- jmp test_all_events
-failsafe_callback:
- GET_CURRENT(%rbx)
- leaq VCPU_trap_bounce(%rbx),%rdx
- movq VCPU_failsafe_addr(%rbx),%rax
- movq %rax,TRAPBOUNCE_eip(%rdx)
- movb $TBF_FAILSAFE,TRAPBOUNCE_flags(%rdx)
- bt $_VGCF_failsafe_disables_events,VCPU_guest_context_flags(%rbx)
- jnc 1f
- orb $TBF_INTERRUPT,TRAPBOUNCE_flags(%rdx)
-1: call create_bounce_frame
- jmp test_all_events
-.previous
- _ASM_PRE_EXTABLE(.Lft0, .Lfx0)
- _ASM_EXTABLE(.Ldf0, failsafe_callback)
+ _ASM_PRE_EXTABLE(.Lft0, handle_exception)
ALIGN
/* No special register assumptions. */
@@ -405,18 +375,7 @@ __UNLIKELY_END(create_bounce_frame_bad_s
subq $8,%rsi
movl TRAPBOUNCE_error_code(%rdx),%eax
.Lft7: movq %rax,(%rsi) # ERROR CODE
-1: testb $TBF_FAILSAFE,%cl
-UNLIKELY_START(nz, bounce_failsafe)
- subq $32,%rsi
- movl %gs,%eax
-.Lft8: movq %rax,24(%rsi) # GS
- movl %fs,%eax
-.Lft9: movq %rax,16(%rsi) # FS
- movl %es,%eax
-.Lft10: movq %rax,8(%rsi) # ES
- movl %ds,%eax
-.Lft11: movq %rax,(%rsi) # DS
-UNLIKELY_END(bounce_failsafe)
+1:
subq $16,%rsi
movq UREGS_r11+8(%rsp),%rax
.Lft12: movq %rax,8(%rsi) # R11
@@ -446,10 +405,6 @@ __UNLIKELY_END(create_bounce_frame_bad_b
_ASM_EXTABLE(.Lft5, domain_crash_page_fault_16)
_ASM_EXTABLE(.Lft6, domain_crash_page_fault)
_ASM_EXTABLE(.Lft7, domain_crash_page_fault)
- _ASM_EXTABLE(.Lft8, domain_crash_page_fault_24)
- _ASM_EXTABLE(.Lft9, domain_crash_page_fault_16)
- _ASM_EXTABLE(.Lft10, domain_crash_page_fault_8)
- _ASM_EXTABLE(.Lft11, domain_crash_page_fault)
_ASM_EXTABLE(.Lft12, domain_crash_page_fault_8)
_ASM_EXTABLE(.Lft13, domain_crash_page_fault)
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -128,7 +128,6 @@
#define TBF_EXCEPTION 1
#define TBF_EXCEPTION_ERRCODE 2
#define TBF_INTERRUPT 8
-#define TBF_FAILSAFE 16
/* 'arch_vcpu' flags values */
#define _TF_kernel_mode 0
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] x86emul: fix rIP handling
2016-02-17 16:32 ` [PATCH 1/5] x86emul: fix rIP handling Jan Beulich
@ 2016-02-17 17:51 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-02-17 17:51 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
On 17/02/16 16:32, Jan Beulich wrote:
> Deal with rIP just like with any other register: Truncate to designated
> width upon entry, write back the zero-extended 32-bit value when
> emulating 32-bit code, and leave the upper 48 bits unchanged for 16-bit
> code.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] x86emul: limit-check branch targets
2016-02-17 16:35 ` [PATCH 2/5] x86emul: limit-check branch targets Jan Beulich
@ 2016-02-17 17:59 ` Andrew Cooper
2016-02-25 14:52 ` Ping: " Jan Beulich
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-02-17 17:59 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: George Dunlap, Tim Deegan, Keir Fraser
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>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] x86emul: simplify IRET logic
2016-02-17 16:36 ` [PATCH 3/5] x86emul: simplify IRET logic Jan Beulich
@ 2016-02-17 18:03 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-02-17 18:03 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
On 17/02/16 16:36, Jan Beulich wrote:
> Since we only handle real mode, we need to consider neither non-ring0
> nor IOPL. Also for POPF the mode_iopl() check can really be inside the
> not-ring-0 body.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] VMX: fold redundant code
2016-02-17 16:37 ` [PATCH 4/5] VMX: fold redundant code Jan Beulich
@ 2016-02-17 18:05 ` Andrew Cooper
2016-02-18 5:26 ` Tian, Kevin
1 sibling, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-02-17 18:05 UTC (permalink / raw)
To: xen-devel
On 17/02/16 16:37, Jan Beulich wrote:
> No need to do this in two slightly different ways, possibly keeping the
> compiler from folding the code for us.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3103,6 +3103,7 @@ void vmx_vmexit_handler(struct cpu_user_
> && vector != TRAP_nmi
> && vector != TRAP_machine_check )
It would be nice if you could nuke the two bits of whitespace after the
TRAP_* constants while you are editing this area.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] x86: drop failsafe callback invocation from assembly
2016-02-17 16:38 ` [PATCH 5/5] x86: drop failsafe callback invocation from assembly Jan Beulich
@ 2016-02-17 19:01 ` Andrew Cooper
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2016-02-17 19:01 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
On 17/02/16 16:38, Jan Beulich wrote:
> Afaict this was never necessary on a 64-bit hypervisor, and was instead
> just blindly cloned over from 32-bit code: We don't fiddle with (and
> hence don't reload) any of DS, ES, FS, or GS, and an exception on IRET
> itself can equally well be reported to the guest as that very exception
> on the target of that IRET.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
As best as I can tell, this looks safe.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
This is one area I intend to write an XTF test for, but I havn't had
time to yet. I will see if I can dig out the start of the test and
complete it.
~Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5] VMX: fold redundant code
2016-02-17 16:37 ` [PATCH 4/5] VMX: fold redundant code Jan Beulich
2016-02-17 18:05 ` Andrew Cooper
@ 2016-02-18 5:26 ` Tian, Kevin
1 sibling, 0 replies; 14+ messages in thread
From: Tian, Kevin @ 2016-02-18 5:26 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Nakajima, Jun
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Thursday, February 18, 2016 12:37 AM
>
> No need to do this in two slightly different ways, possibly keeping the
> compiler from folding the code for us.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Ping: [PATCH 2/5] x86emul: limit-check branch targets
2016-02-17 16:35 ` [PATCH 2/5] x86emul: limit-check branch targets Jan Beulich
2016-02-17 17:59 ` Andrew Cooper
@ 2016-02-25 14:52 ` Jan Beulich
2016-02-26 9:44 ` Tim Deegan
1 sibling, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2016-02-25 14:52 UTC (permalink / raw)
To: Tim Deegan; +Cc: George Dunlap, Andrew Cooper, Keir Fraser, xen-devel
>>> 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);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Ping: [PATCH 2/5] x86emul: limit-check branch targets
2016-02-25 14:52 ` Ping: " Jan Beulich
@ 2016-02-26 9:44 ` Tim Deegan
0 siblings, 0 replies; 14+ messages in thread
From: Tim Deegan @ 2016-02-26 9:44 UTC (permalink / raw)
To: Jan Beulich; +Cc: George Dunlap, Andrew Cooper, Keir Fraser, xen-devel
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>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-02-26 9:44 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-17 16:21 [PATCH 0/5] x86: XSA-170 follow-up Jan Beulich
2016-02-17 16:32 ` [PATCH 1/5] x86emul: fix rIP handling Jan Beulich
2016-02-17 17:51 ` Andrew Cooper
2016-02-17 16:35 ` [PATCH 2/5] x86emul: limit-check branch targets Jan Beulich
2016-02-17 17:59 ` Andrew Cooper
2016-02-25 14:52 ` Ping: " Jan Beulich
2016-02-26 9:44 ` Tim Deegan
2016-02-17 16:36 ` [PATCH 3/5] x86emul: simplify IRET logic Jan Beulich
2016-02-17 18:03 ` Andrew Cooper
2016-02-17 16:37 ` [PATCH 4/5] VMX: fold redundant code Jan Beulich
2016-02-17 18:05 ` Andrew Cooper
2016-02-18 5:26 ` Tian, Kevin
2016-02-17 16:38 ` [PATCH 5/5] x86: drop failsafe callback invocation from assembly Jan Beulich
2016-02-17 19:01 ` Andrew Cooper
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.