[PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Avi Kivity]

If the guest thinks it's an AMD, it will not have prepared the SYSENTER MSRs,
and if the guest executes SYSENTER in compatibility mode, it will fails.

Detect this condition and #UD instead, like the spec says.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 6eaedac..367bd06 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1892,6 +1892,16 @@ static int em_lseg(struct x86_emulate_ctxt *ctxt)
 	ss->p = 1;
 }
 
+static bool vendor_intel(struct x86_emulate_ctxt *ctxt)
+{
+	u32 eax, ebx, ecx, edx;
+
+	return ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx)
+		&& ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx
+		&& ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx
+		&& edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx;
+}
+
 static bool em_syscall_is_enabled(struct x86_emulate_ctxt *ctxt)
 {
 	struct x86_emulate_ops *ops = ctxt->ops;
@@ -2008,6 +2018,14 @@ static int em_sysenter(struct x86_emulate_ctxt *ctxt)
 	if (ctxt->mode == X86EMUL_MODE_REAL)
 		return emulate_gp(ctxt, 0);
 
+	/*
+	 * Not recognized on AMD in compat mode (but is recognized in legacy
+	 * mode).
+	 */
+	if ((ctxt->mode == X86EMUL_MODE_PROT32) && (efer & EFER_LMA)
+	    && !vendor_intel(ctxt))
+		return emulate_ud(ctxt);
+
 	/* XXX sysenter/sysexit have not been tested in 64bit mode.
 	* Therefore, we inject an #UD.
 	*/
-- 
1.7.9

Re: [PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Stephan Bärwolf]

On 02/01/12 11:27, Avi Kivity wrote:
>  
> +static bool vendor_intel(struct x86_emulate_ctxt *ctxt)
> +{
> +	u32 eax, ebx, ecx, edx;
Just to be clean:

Shouldn't eax and ecx be initialized to zero?

regards Stephan
> +
> +	return ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx)
> +		&& ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx
> +		&& ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx
> +		&& edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx;
> +}
> +

Re: [PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Avi Kivity]

On 02/01/2012 12:45 PM, Stephan Bärwolf wrote:
> On 02/01/12 11:27, Avi Kivity wrote:
> >  
> > +static bool vendor_intel(struct x86_emulate_ctxt *ctxt)
> > +{
> > +	u32 eax, ebx, ecx, edx;
> Just to be clean:
>
> Shouldn't eax and ecx be initialized to zero?

Either get_cpuid() initializes them and returns true, or it returns
false and we never evaluate them.

> regards Stephan
> > +
> > +	return ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx)
> > +		&& ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx
> > +		&& ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx
> > +		&& edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx;
> > +}
> > +
>




-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Stephan Bärwolf]

On 02/01/12 11:50, Avi Kivity wrote:
> On 02/01/2012 12:45 PM, Stephan Bärwolf wrote:
>> On 02/01/12 11:27, Avi Kivity wrote:
>>>  
>>> +static bool vendor_intel(struct x86_emulate_ctxt *ctxt)
>>> +{
>>> +	u32 eax, ebx, ecx, edx;
>> Just to be clean:
>>
>> Shouldn't eax and ecx be initialized to zero?
> Either get_cpuid() initializes them and returns true, or it returns
> false and we never evaluate them.
This isn't what I mean.

CPUID will only return back the VENDOR (in ebx..edx) if you call it
with eax=0x00000000 and ecx=0x00000000 (see cpu docs).

Of course the compiler should set these two vars to zero (?)
But what if somebody wants to read this code sometime later ??
(The compiler will optimize the "set to zero" away...)

regards Stephan

Re: [PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Avi Kivity]

On 02/01/2012 12:59 PM, Stephan Bärwolf wrote:
> On 02/01/12 11:50, Avi Kivity wrote:
> > On 02/01/2012 12:45 PM, Stephan Bärwolf wrote:
> >> On 02/01/12 11:27, Avi Kivity wrote:
> >>>  
> >>> +static bool vendor_intel(struct x86_emulate_ctxt *ctxt)
> >>> +{
> >>> +	u32 eax, ebx, ecx, edx;
> >> Just to be clean:
> >>
> >> Shouldn't eax and ecx be initialized to zero?
> > Either get_cpuid() initializes them and returns true, or it returns
> > false and we never evaluate them.
> This isn't what I mean.
>
> CPUID will only return back the VENDOR (in ebx..edx) if you call it
> with eax=0x00000000 and ecx=0x00000000 (see cpu docs).
>
> Of course the compiler should set these two vars to zero (?)
> But what if somebody wants to read this code sometime later ??
> (The compiler will optimize the "set to zero" away...)
>

Ah, right of course.  Will update.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Marcelo Tosatti]

On Wed, Feb 01, 2012 at 12:27:00PM +0200, Avi Kivity wrote:
> If the guest thinks it's an AMD, it will not have prepared the SYSENTER MSRs,
> and if the guest executes SYSENTER in compatibility mode, it will fails.
> 
> Detect this condition and #UD instead, like the spec says.

Note the SYSENTER_CS & 0xfffc == 0 check protects against a fatal error (unlike
the SYSCALL bug), in case of zeroed MSRs.

Re: [PATCH] KVM: x86 emulator: reject SYSENTER in compatibility mode on AMD guests

[Avi Kivity]

On 02/01/2012 01:00 PM, Marcelo Tosatti wrote:
> On Wed, Feb 01, 2012 at 12:27:00PM +0200, Avi Kivity wrote:
> > If the guest thinks it's an AMD, it will not have prepared the SYSENTER MSRs,
> > and if the guest executes SYSENTER in compatibility mode, it will fails.
> > 
> > Detect this condition and #UD instead, like the spec says.
>
> Note the SYSENTER_CS & 0xfffc == 0 check protects against a fatal error (unlike
> the SYSCALL bug), in case of zeroed MSRs.

That's a relief.  It's still possible for this to fail (say kexec from
32-bit guest to 64-bit guest) but it's incredibly unlikely.

-- 
error compiling committee.c: too many arguments to function