[PATCH] KVM: VMX: Fix host MSR_KERNEL_GS_BASE corruption

[PATCH] KVM: VMX: Fix host MSR_KERNEL_GS_BASE corruption

[Avi Kivity]

enter_lmode() and exit_lmode() modify the guest's EFER.LMA before calling
vmx_set_efer().  However, the latter function depends on the value of EFER.LMA
to determine whether MSR_KERNEL_GS_BASE needs reloading, via
vmx_load_host_state().  With EFER.LMA changing under its feet, it took the
wrong choice and corrupted userspace's %gs.

This causes 32-on-64 host userspace to fault.

Fix not touching EFER.LMA; instead ask vmx_set_efer() to change it.

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

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ebaaeaf..2fdcc98 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1812,18 +1812,15 @@ static void enter_lmode(struct kvm_vcpu *vcpu)
 			     (guest_tr_ar & ~AR_TYPE_MASK)
 			     | AR_TYPE_BUSY_64_TSS);
 	}
-	vcpu->arch.efer |= EFER_LMA;
-	vmx_set_efer(vcpu, vcpu->arch.efer);
+	vmx_set_efer(vcpu, vcpu->arch.efer | EFER_LMA);
 }
 
 static void exit_lmode(struct kvm_vcpu *vcpu)
 {
-	vcpu->arch.efer &= ~EFER_LMA;
-
 	vmcs_write32(VM_ENTRY_CONTROLS,
 		     vmcs_read32(VM_ENTRY_CONTROLS)
 		     & ~VM_ENTRY_IA32E_MODE);
-	vmx_set_efer(vcpu, vcpu->arch.efer);
+	vmx_set_efer(vcpu, vcpu->arch.efer & ~EFER_LMA);
 }
 
 #endif
-- 
1.7.1

Re: [PATCH] KVM: VMX: Fix host MSR_KERNEL_GS_BASE corruption

[Marcelo Tosatti]

On Tue, Jul 06, 2010 at 11:36:01AM +0300, Avi Kivity wrote:
> enter_lmode() and exit_lmode() modify the guest's EFER.LMA before calling
> vmx_set_efer().  However, the latter function depends on the value of EFER.LMA
> to determine whether MSR_KERNEL_GS_BASE needs reloading, via
> vmx_load_host_state().  With EFER.LMA changing under its feet, it took the
> wrong choice and corrupted userspace's %gs.
> 
> This causes 32-on-64 host userspace to fault.
> 
> Fix not touching EFER.LMA; instead ask vmx_set_efer() to change it.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/vmx.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ebaaeaf..2fdcc98 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1812,18 +1812,15 @@ static void enter_lmode(struct kvm_vcpu *vcpu)
>  			     (guest_tr_ar & ~AR_TYPE_MASK)
>  			     | AR_TYPE_BUSY_64_TSS);
>  	}
> -	vcpu->arch.efer |= EFER_LMA;
> -	vmx_set_efer(vcpu, vcpu->arch.efer);
> +	vmx_set_efer(vcpu, vcpu->arch.efer | EFER_LMA);
>  }
>  
>  static void exit_lmode(struct kvm_vcpu *vcpu)
>  {
> -	vcpu->arch.efer &= ~EFER_LMA;
> -
>  	vmcs_write32(VM_ENTRY_CONTROLS,
>  		     vmcs_read32(VM_ENTRY_CONTROLS)
>  		     & ~VM_ENTRY_IA32E_MODE);
> -	vmx_set_efer(vcpu, vcpu->arch.efer);
> +	vmx_set_efer(vcpu, vcpu->arch.efer & ~EFER_LMA);
>  }
>  
>  #endif
> -- 
> 1.7.1

Looks good.