* [PATCH 1/2] x86emul: deal with ASSERT()s triggering
2017-01-25 10:28 [PATCH 0/2] x86emul: deal with ASSERT()s triggering Jan Beulich
@ 2017-01-25 10:31 ` Jan Beulich
2017-01-25 10:33 ` Andrew Cooper
2017-01-25 10:32 ` [PATCH 2/2] x86/emulate: don't assume that addr_size == 32 implies protected mode Jan Beulich
1 sibling, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2017-01-25 10:31 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
Operand size defaults to 32 bits in that case, but would not have been
set that way in the absence of an operand size override.
Reported-by: Wei Liu <wei.liu2@citrix.com>
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
@@ -2298,6 +2298,11 @@ x86_decode(
case 8:
/* VEX / XOP / EVEX */
generate_exception_if(rex_prefix || vex.pfx, EXC_UD);
+ /*
+ * With operand size override disallowed (see above), op_bytes
+ * should not have changed from its default.
+ */
+ ASSERT(op_bytes == def_op_bytes);
vex.raw[0] = modrm;
if ( b == 0xc5 )
@@ -2326,7 +2331,8 @@ x86_decode(
}
else
{
- ASSERT(op_bytes == 4);
+ /* Operand size fixed at 4 (no override via W bit). */
+ op_bytes = 4;
vex.b = 1;
}
switch ( b )
[-- Attachment #2: x86emul-VEX-16bit.patch --]
[-- Type: text/plain, Size: 1251 bytes --]
x86emul: correct VEX/XOP/EVEX operand size handling for 16-bit code
Operand size defaults to 32 bits in that case, but would not have been
set that way in the absence of an operand size override.
Reported-by: Wei Liu <wei.liu2@citrix.com>
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
@@ -2298,6 +2298,11 @@ x86_decode(
case 8:
/* VEX / XOP / EVEX */
generate_exception_if(rex_prefix || vex.pfx, EXC_UD);
+ /*
+ * With operand size override disallowed (see above), op_bytes
+ * should not have changed from its default.
+ */
+ ASSERT(op_bytes == def_op_bytes);
vex.raw[0] = modrm;
if ( b == 0xc5 )
@@ -2326,7 +2331,8 @@ x86_decode(
}
else
{
- ASSERT(op_bytes == 4);
+ /* Operand size fixed at 4 (no override via W bit). */
+ op_bytes = 4;
vex.b = 1;
}
switch ( b )
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] x86/emulate: don't assume that addr_size == 32 implies protected mode
2017-01-25 10:28 [PATCH 0/2] x86emul: deal with ASSERT()s triggering Jan Beulich
2017-01-25 10:31 ` [PATCH 1/2] " Jan Beulich
@ 2017-01-25 10:32 ` Jan Beulich
2017-01-25 10:38 ` Andrew Cooper
1 sibling, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2017-01-25 10:32 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu
[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]
From: George Dunlap <george.dunlap@citrix.com>
Callers of x86_emulate() generally define addr_size based on the code
segment. In vm86 mode, the code segment is set by the hardware to be
16-bits; but it is entirely possible to enable protected mode, set the
CS to 32-bits, and then disable protected mode. (This is commonly
called "unreal mode".)
But the instruction decoder only checks for protected mode when
addr_size == 16. So in unreal mode, hardware will throw a #UD for VEX
prefixes, but our instruction decoder will decode them, triggering an
ASSERT() further on in _get_fpu(). (With debug=n the emulator will
incorrectly emulate the instruction rather than throwing a #UD, but
this is only a bug, not a crash, so it's not a security issue.)
Teach the instruction decoder to check that we're in protected mode,
even if addr_size is 32.
While we're here, replace the open-coded protected mode check with
in_protmode().
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Split real mode and VM86 mode handling, as VM86 mode is strictly 16-bit
at all times. Re-base.
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
@@ -2288,11 +2288,11 @@ x86_decode(
default:
BUG(); /* Shouldn't be possible. */
case 2:
- if ( in_realmode(ctxt, ops) || (state->regs->_eflags & EFLG_VM) )
+ if ( state->regs->_eflags & EFLG_VM )
break;
/* fall through */
case 4:
- if ( modrm_mod != 3 )
+ if ( modrm_mod != 3 || in_realmode(ctxt, ops) )
break;
/* fall through */
case 8:
[-- Attachment #2: x86emul-unreal-mode.patch --]
[-- Type: text/plain, Size: 1836 bytes --]
x86/emulate: don't assume that addr_size == 32 implies protected mode
Callers of x86_emulate() generally define addr_size based on the code
segment. In vm86 mode, the code segment is set by the hardware to be
16-bits; but it is entirely possible to enable protected mode, set the
CS to 32-bits, and then disable protected mode. (This is commonly
called "unreal mode".)
But the instruction decoder only checks for protected mode when
addr_size == 16. So in unreal mode, hardware will throw a #UD for VEX
prefixes, but our instruction decoder will decode them, triggering an
ASSERT() further on in _get_fpu(). (With debug=n the emulator will
incorrectly emulate the instruction rather than throwing a #UD, but
this is only a bug, not a crash, so it's not a security issue.)
Teach the instruction decoder to check that we're in protected mode,
even if addr_size is 32.
While we're here, replace the open-coded protected mode check with
in_protmode().
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Split real mode and VM86 mode handling, as VM86 mode is strictly 16-bit
at all times. Re-base.
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
@@ -2288,11 +2288,11 @@ x86_decode(
default:
BUG(); /* Shouldn't be possible. */
case 2:
- if ( in_realmode(ctxt, ops) || (state->regs->_eflags & EFLG_VM) )
+ if ( state->regs->_eflags & EFLG_VM )
break;
/* fall through */
case 4:
- if ( modrm_mod != 3 )
+ if ( modrm_mod != 3 || in_realmode(ctxt, ops) )
break;
/* fall through */
case 8:
[-- Attachment #3: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread