Re: [PATCH] KVM: vmx: VMXOFF emulation in vm86 should cause #UD
Il 29/08/2014 10:26, Nadav Amit ha scritto:
Unlike VMCALL, the instructions VMXOFF, VMLAUNCH and VMRESUME should cause a
UD
exception in real-mode or vm86. However, the emulator considers all these
instructions the same for the matter of mode checks, and emulation upon exit
due to #UD exception.
As a result, the hypervisor behaves incorrectly on vm86 mode. VMXOFF, VMLAUNCH
or VMRESUME cause on vm86 exit due to #UD. The hypervisor then emulates these
instruction and inject #GP to the guest instead of #UD.
This patch creates a new group for these instructions and mark only VMCALL as
an instruction which can be emulated.
Signed-off-by: Nadav Amit na...@cs.technion.ac.il
Patch looks good, but where is the check that MOD == 3 in the case
RMExt? Am I just not seeing it?
Paolo
---
arch/x86/kvm/emulate.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e5bf130..a240fac 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3139,12 +3139,8 @@ static int em_clts(struct x86_emulate_ctxt *ctxt)
static int em_vmcall(struct x86_emulate_ctxt *ctxt)
{
- int rc;
-
- if (ctxt-modrm_mod != 3 || ctxt-modrm_rm != 1)
- return X86EMUL_UNHANDLEABLE;
+ int rc = ctxt-ops-fix_hypercall(ctxt);
- rc = ctxt-ops-fix_hypercall(ctxt);
if (rc != X86EMUL_CONTINUE)
return rc;
@@ -3562,6 +3558,12 @@ static int check_perm_out(struct x86_emulate_ctxt
*ctxt)
F2bv(((_f) | DstReg | SrcMem | ModRM) ~Lock, _e), \
F2bv(((_f) ~Lock) | DstAcc | SrcImm, _e)
+static const struct opcode group7_rm0[] = {
+ N,
+ I(SrcNone | Priv | EmulateOnUD, em_vmcall),
+ N, N, N, N, N, N,
+};
+
static const struct opcode group7_rm1[] = {
DI(SrcNone | Priv, monitor),
DI(SrcNone | Priv, mwait),
@@ -3655,7 +3657,7 @@ static const struct group_dual group7 = { {
II(SrcMem16 | Mov | Priv, em_lmsw, lmsw),
II(SrcMem | ByteOp | Priv | NoAccess, em_invlpg, invlpg),
}, {
- I(SrcNone | Priv | EmulateOnUD, em_vmcall),
+ EXT(0, group7_rm0),
EXT(0, group7_rm1),
N, EXT(0, group7_rm3),
II(SrcNone | DstMem | Mov, em_smsw, smsw), N,
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: vmx: VMXOFF emulation in vm86 should cause #UD
On Aug 29, 2014, at 11:36 AM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 29/08/2014 10:26, Nadav Amit ha scritto:
Unlike VMCALL, the instructions VMXOFF, VMLAUNCH and VMRESUME should cause a
UD
exception in real-mode or vm86. However, the emulator considers all these
instructions the same for the matter of mode checks, and emulation upon exit
due to #UD exception.
As a result, the hypervisor behaves incorrectly on vm86 mode. VMXOFF,
VMLAUNCH
or VMRESUME cause on vm86 exit due to #UD. The hypervisor then emulates these
instruction and inject #GP to the guest instead of #UD.
This patch creates a new group for these instructions and mark only VMCALL as
an instruction which can be emulated.
Signed-off-by: Nadav Amit na...@cs.technion.ac.il
Patch looks good, but where is the check that MOD == 3 in the case
RMExt? Am I just not seeing it?
This seems to be part of the “case GroupDual”.
---
arch/x86/kvm/emulate.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index e5bf130..a240fac 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3139,12 +3139,8 @@ static int em_clts(struct x86_emulate_ctxt *ctxt)
static int em_vmcall(struct x86_emulate_ctxt *ctxt)
{
-int rc;
-
-if (ctxt-modrm_mod != 3 || ctxt-modrm_rm != 1)
-return X86EMUL_UNHANDLEABLE;
+int rc = ctxt-ops-fix_hypercall(ctxt);
-rc = ctxt-ops-fix_hypercall(ctxt);
if (rc != X86EMUL_CONTINUE)
return rc;
@@ -3562,6 +3558,12 @@ static int check_perm_out(struct x86_emulate_ctxt
*ctxt)
F2bv(((_f) | DstReg | SrcMem | ModRM) ~Lock, _e), \
F2bv(((_f) ~Lock) | DstAcc | SrcImm, _e)
+static const struct opcode group7_rm0[] = {
+N,
+I(SrcNone | Priv | EmulateOnUD, em_vmcall),
+N, N, N, N, N, N,
+};
+
static const struct opcode group7_rm1[] = {
DI(SrcNone | Priv, monitor),
DI(SrcNone | Priv, mwait),
@@ -3655,7 +3657,7 @@ static const struct group_dual group7 = { {
II(SrcMem16 | Mov | Priv, em_lmsw, lmsw),
II(SrcMem | ByteOp | Priv | NoAccess, em_invlpg, invlpg),
}, {
-I(SrcNone | Priv | EmulateOnUD, em_vmcall),
+EXT(0, group7_rm0),
EXT(0, group7_rm1),
N, EXT(0, group7_rm3),
II(SrcNone | DstMem | Mov, em_smsw, smsw), N,
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: vmx: VMXOFF emulation in vm86 should cause #UD
Il 29/08/2014 10:52, Nadav Amit ha scritto: Patch looks good, but where is the check that MOD == 3 in the case RMExt? Am I just not seeing it? This seems to be part of the “case GroupDual”. GroupDual handles it, but the EXT() macro you're using is exactly what you want: #define RMExt (415) /* Opcode extension in ModRM r/m if mod == 3 */ I guess what's missing is -- 8 -- Subject: [PATCH] Check ModRM for RMExt Some group7 extensions are only defined for mod==3. Check this and reject emulation if mod!=3. Signed-off-by: Paolo Bonzini pbonz...@redhat.com diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 56657b0bb3bb..d472e4d50e3c 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4360,6 +4360,8 @@ done_prefixes: opcode = opcode.u.gdual-mod012[goffset]; break; case RMExt: + if ((ctxt-modrm 6) == 3) + return EMULATION_FAILED; goffset = ctxt-modrm 7; opcode = opcode.u.group[goffset]; break; What do you think? Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: vmx: VMXOFF emulation in vm86 should cause #UD
On Aug 29, 2014, at 11:57 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 29/08/2014 10:52, Nadav Amit ha scritto: Patch looks good, but where is the check that MOD == 3 in the case RMExt? Am I just not seeing it? This seems to be part of the “case GroupDual”. GroupDual handles it, but the EXT() macro you're using is exactly what you want: #define RMExt (415) /* Opcode extension in ModRM r/m if mod == 3 */ I guess what's missing is -- 8 -- Subject: [PATCH] Check ModRM for RMExt Some group7 extensions are only defined for mod==3. Check this and reject emulation if mod!=3. Signed-off-by: Paolo Bonzini pbonz...@redhat.com diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 56657b0bb3bb..d472e4d50e3c 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -4360,6 +4360,8 @@ done_prefixes: opcode = opcode.u.gdual-mod012[goffset]; break; case RMExt: + if ((ctxt-modrm 6) == 3) + return EMULATION_FAILED; goffset = ctxt-modrm 7; opcode = opcode.u.group[goffset]; break; What do you think? I don’t know. I am looking at Intel SDM table A-6 (Opcode Extensions for One- and Two-byte Opcodes by Group Number). According to the table, only group 7 needs RMExt, and in this case the “case GroupDual” makes the required checks, in the iteration prior to the “case RMExt”. Therefore this code path, RMExt without GroupDual before it, should never occur. Nonetheless, if you want to avoid future bugs, perhaps it is good. Nadav signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH] KVM: vmx: VMXOFF emulation in vm86 should cause #UD
Il 29/08/2014 11:12, Nadav Amit ha scritto: I don’t know. I am looking at Intel SDM table A-6 (Opcode Extensions for One- and Two-byte Opcodes by Group Number). According to the table, only group 7 needs RMExt, and in this case the “case GroupDual” makes the required checks, in the iteration prior to the “case RMExt”. Therefore this code path, RMExt without GroupDual before it, should never occur. Nonetheless, if you want to avoid future bugs, perhaps it is good. Oh, now I understand what you mean. Thanks, Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
