From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <4F6C1FC3.3090709@canonical.com> Date: Fri, 23 Mar 2012 08:01:23 +0100 From: Stefan Bader MIME-Version: 1.0 To: Marcelo Tosatti CC: stable@vger.kernel.org, kvm@vger.kernel.org, Stephan Baerwolf , Avi Kivity Subject: Re: [v2.6.32.y 2/2] KVM: x86: fix missing checks in syscall emulation References: <1332406246-3978-1-git-send-email-stefan.bader@canonical.com> <1332406246-3978-3-git-send-email-stefan.bader@canonical.com> <20120323000722.GA28274@amt.cnet> In-Reply-To: <20120323000722.GA28274@amt.cnet> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: kvm-owner@vger.kernel.org List-ID: -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 23.03.2012 01:07, Marcelo Tosatti wrote: > On Thu, Mar 22, 2012 at 09:50:42AM +0100, Stefan Bader wrote: >>> From 69712f0c7cbb6363f7b2170fba93945a72d77712 Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?Stephan=20B=C3=A4rwolf?= >> Date: Thu, 12 Jan 2012 16:43:04 +0100 >> Subject: [PATCH 2/2] KVM: x86: fix missing checks in syscall emulation >> >> On hosts without this patch, 32bit guests will crash (and 64bit guests >> may behave in a wrong way) for example by simply executing following >> nasm-demo-application: >> >> [bits 32] global _start SECTION .text _start: syscall >> >> (I tested it with winxp and linux - both always crashed) >> >> Disassembly of section .text: >> >> 00000000 <_start>: 0: 0f 05 syscall >> >> The reason seems a missing "invalid opcode"-trap (int6) for the syscall >> opcode "0f05", which is not available on Intel CPUs within non-longmodes, >> as also on some AMD CPUs within legacy-mode. (depending on CPU vendor, >> MSR_EFER and cpuid) >> >> Because previous mentioned OSs may not engage corresponding syscall >> target-registers (STAR, LSTAR, CSTAR), they remain NULL and (non >> trapping) syscalls are leading to multiple faults and finally crashs. >> >> Depending on the architecture (AMD or Intel) pretended by guests, various >> checks according to vendor's documentation are implemented to overcome >> the current issue and behave like the CPUs physical counterparts. >> >> [mtosatti: cleanup/beautify code] >> >> Signed-off-by: Stephan Baerwolf >> Signed-off-by: Marcelo Tosatti >> >> (backported from commit c2226fc9e87ba3da060e47333657cd6616652b84 >> upstream) Signed-off-by: Stefan Bader --- >> arch/x86/include/asm/kvm_emulate.h | 13 ++++++++ arch/x86/kvm/emulate.c >> | 57 ++++++++++++++++++++++++++++++++++- 2 files changed, 68 >> insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/include/asm/kvm_emulate.h >> b/arch/x86/include/asm/kvm_emulate.h index 5d938f9..1f137af 100644 --- >> a/arch/x86/include/asm/kvm_emulate.h +++ >> b/arch/x86/include/asm/kvm_emulate.h @@ -185,6 +185,19 @@ struct >> x86_emulate_ctxt { #define X86EMUL_MODE_PROT32 4 /* 32-bit protected >> mode. */ #define X86EMUL_MODE_PROT64 8 /* 64-bit (long) mode. */ >> >> +/* CPUID vendors */ +#define X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx >> 0x68747541 +#define X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx 0x444d4163 >> +#define X86EMUL_CPUID_VENDOR_AuthenticAMD_edx 0x69746e65 + +#define >> X86EMUL_CPUID_VENDOR_AMDisbetterI_ebx 0x69444d41 +#define >> X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx 0x21726574 +#define >> X86EMUL_CPUID_VENDOR_AMDisbetterI_edx 0x74656273 + +#define >> X86EMUL_CPUID_VENDOR_GenuineIntel_ebx 0x756e6547 +#define >> X86EMUL_CPUID_VENDOR_GenuineIntel_ecx 0x6c65746e +#define >> X86EMUL_CPUID_VENDOR_GenuineIntel_edx 0x49656e69 + /* Host execution >> mode. */ #if defined(CONFIG_X86_32) #define X86EMUL_MODE_HOST >> X86EMUL_MODE_PROT32 diff --git a/arch/x86/kvm/emulate.c >> b/arch/x86/kvm/emulate.c index 1350e43..10134d2 100644 --- >> a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1495,20 >> +1495,73 @@ setup_syscalls_segments(struct x86_emulate_ctxt *ctxt, >> ss->present = 1; } >> >> +static bool syscall_is_enabled(struct x86_emulate_ctxt *ctxt, + >> struct x86_emulate_ops *ops) +{ + u32 eax, ebx, ecx, edx; + + /* + * >> syscall should always be enabled in longmode - so only become + * vendor >> specific (cpuid) if other modes are active... + */ + if (ctxt->mode == >> X86EMUL_MODE_PROT64) + return true; + + eax = 0x00000000; + ecx = >> 0x00000000; + if (ops->get_cpuid(ctxt->vcpu, &eax, &ebx, &ecx, &edx)) { + >> /* + * Intel ("GenuineIntel") + * remark: Intel CPUs only support >> "syscall" in 64bit + * longmode. Also an 64bit guest with a + * 32bit >> compat-app running will #UD !! While this + * behaviour can be fixed >> (by emulating) into AMD + * response - CPUs of AMD can't behave like >> Intel. + */ + if (ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx && + >> ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx && + edx == >> X86EMUL_CPUID_VENDOR_GenuineIntel_edx) + return false; + + /* AMD >> ("AuthenticAMD") */ + if (ebx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx >> && + ecx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx && + edx == >> X86EMUL_CPUID_VENDOR_AuthenticAMD_edx) + return true; + + /* AMD >> ("AMDisbetter!") */ + if (ebx == X86EMUL_CPUID_VENDOR_AMDisbetterI_ebx >> && + ecx == X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx && + edx == >> X86EMUL_CPUID_VENDOR_AMDisbetterI_edx) + return true; + } + + /* >> default: (not Intel, not AMD), apply Intel's stricter rules... */ + >> return false; +} + static int -emulate_syscall(struct x86_emulate_ctxt >> *ctxt) +emulate_syscall(struct x86_emulate_ctxt *ctxt, struct >> x86_emulate_ops *ops) { struct decode_cache *c = &ctxt->decode; struct >> kvm_segment cs, ss; u64 msr_data; + u64 efer = 0; >> >> /* syscall is not available in real mode */ if (c->lock_prefix || >> ctxt->mode == X86EMUL_MODE_REAL || ctxt->mode == X86EMUL_MODE_VM86) >> return -1; >> >> + if (!(syscall_is_enabled(ctxt, ops))) + return -1; + + >> kvm_x86_ops->get_msr(ctxt->vcpu, MSR_EFER, &efer); >> setup_syscalls_segments(ctxt, &cs, &ss); >> >> + if (!(efer & EFER_SCE)) + return -1; + > > It should inject an exception (kvm_queue_exception(UD_VECTOR)) instead of > simply looping on the same instruction. > > Hm, ok. I did not want to exit differently from what is done in the real mode case. Practically (in testing) it seemed to end up having the same effect on the test case. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQIcBAEBCgAGBQJPbB+6AAoJEOhnXe7L7s6jJUQP/39+Q9YQxVsd4A/SH0R7XDOS 2YWwrQLkt3JldRD/sYCj4GY8x3r3cdCZud0dBpML8GkiP65dryc9momrQJquKNwI cDxXMnZucMQCUV1hDL/P0VrB3Mfvj3eVn4xZlSNgkSdmFmFXRpiiA04EVdHAVx1H f7VWIDNA+wISH59nCwzV4lOv/CrUx2MBkYBNcoS3BKmkiyftn/WFLIZA7+T9tJ2Q /kzCp+KHEipU30CGR3lEuCdSWf1GrhRIZlqdKx9JzVIXmmVMy9olNz1GHbu3agXm t4JIjHUlxtqfB9G1VQZWgy18vcUJWlzO/WWFtv5YHRWE1qXv1JdI1vEKaYzMMZVP Io4rCSeuh/lXwdO+DM344KZQ+CzQv0ZyCd14O13gUt+zrCcuMvEye1UH/7D9pSw2 9WBEcj7hQLcSkAYgLHqWnuoNmjqGbzr/5GmGkfQ1+JB91kS7yA4NzbbVbTZI+oOK BBkqIwy4702pf2wAUeyVFo990O4jF8nszbIqoCvh2N7JdMow30D6/iPN6JKsz2D5 vJGKOmGRlS0CkmCHRM0BaZc7TYqnd9Gg8EOirHVgxi6XUCHXMO7W0gpIGwWQU3Jr 5rxkuEctallXuTgZ7R1sOxeL5+v4hCHIGqMRgX1j3O3ThyZ6fYa7Ii/c6mDNlfle ptHXJcpKOfs7sYeYg0w5 =7nbx -----END PGP SIGNATURE-----