Re: [v2.6.32.y 2/2] KVM: x86: fix missing checks in syscall emulation

[Marcelo Tosatti <mtosatti@redhat.com>]

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?= <stephan.baerwolf@tu-ilmenau.de>
> 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 <stephan.baerwolf@tu-ilmenau.de>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> (backported from commit c2226fc9e87ba3da060e47333657cd6616652b84 upstream)
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  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.