Fwd: Re: [PATCH master/3.5.y] KVM: VMX: Fix ds/es corruption on i386 with preemption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



HI,

I've just been looking through the patches Greg KH has stored for 3.5-stable and this doesn't seem to be amongst them.

I can't see any way in which it contravenes the stable rules and, given there's a distinct possibility of a mix of i386, kvm-1.1.x and a 3.5 kernel resulting in a crash, it seems to me to be a good candidate. The commit in mainline is aa67f6096c19bcdb1951ef88be3cf3d2118809dc.

Thanks.


-------- Original Message --------
Subject: Re: [PATCH master/3.5.y] KVM: VMX: Fix ds/es corruption on i386 with preemption
Date: Wed, 01 Aug 2012 15:36:30 +0100
From: Chris Clayton <chris2553@xxxxxxxxxxxxxx>
To: Avi Kivity <avi@xxxxxxxxxx>
CC: Marcelo Tosatti <mtosatti@xxxxxxxxxx>, kvm@xxxxxxxxxxxxxxx

On 08/01/12 14:48, Avi Kivity wrote:
Commit b2da15ac26a0c ("KVM: VMX: Optimize %ds, %es reload") broke i386
in the following scenario:

   vcpu_load
   ...
   vmx_save_host_state
   vmx_vcpu_run
   (ds.rpl, es.rpl cleared by hardware)

   interrupt
     push ds, es  # pushes bad ds, es
     schedule
       vmx_vcpu_put
         vmx_load_host_state
           reload ds, es (with __USER_DS)
     pop ds, es  # of other thread's stack
     iret
   # other thread runs
   interrupt
     push ds, es
     schedule  # back in vcpu thread
     pop ds, es  # now with rpl=0
     iret
   ...
   vcpu_put
   resume_userspace
   iret  # clears ds, es due to mismatched rpl

(instead of resume_userspace, we might return with SYSEXIT and then
take an exception; when the exception IRETs we end up with cleared
ds, es)

Fix by avoiding the optimization on i386 and reloading ds, es on the
lightweight exit path.

Reported-by: Chris Clayron <chris2553@xxxxxxxxxxxxxx>
Signed-off-by: Avi Kivity <avi@xxxxxxxxxx>

I've just had 15 successful runs of qemu-kvm-1.1.1 with kernel 3.5.0
plus this patch, so:

Tested-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx>

I assume this will be forwarded to stable once it has been applied to
mainline.

---
  arch/x86/kvm/vmx.c | 20 +++++++++++++-------
  1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c39b607..c00f03d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1488,13 +1488,6 @@ static void __vmx_load_host_state(struct vcpu_vmx *vmx)
  		loadsegment(ds, vmx->host_state.ds_sel);
  		loadsegment(es, vmx->host_state.es_sel);
  	}
-#else
-	/*
-	 * The sysexit path does not restore ds/es, so we must set them to
-	 * a reasonable value ourselves.
-	 */
-	loadsegment(ds, __USER_DS);
-	loadsegment(es, __USER_DS);
  #endif
  	reload_tss();
  #ifdef CONFIG_X86_64
@@ -6370,6 +6363,19 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
  #endif
  	      );

+#ifndef CONFIG_X86_64
+	/*
+	 * The sysexit path does not restore ds/es, so we must set them to
+	 * a reasonable value ourselves.
+	 *
+	 * We can't defer this to vmx_load_host_state() since that function
+	 * may be executed in interrupt context, which saves and restore segments
+	 * around it, nullifying its effect.
+	 */
+	loadsegment(ds, __USER_DS);
+	loadsegment(es, __USER_DS);
+#endif
+
  	vcpu->arch.regs_avail = ~((1 << VCPU_REGS_RIP) | (1 << VCPU_REGS_RSP)
  				  | (1 << VCPU_EXREG_RFLAGS)
  				  | (1 << VCPU_EXREG_CPL)






--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux