From: Greg KH <[email protected]>

3.5-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Avi Kivity <[email protected]>

(cherry picked from commit aa67f6096c19bcdb1951ef88be3cf3d2118809dc)

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 <[email protected]>
Signed-off-by: Avi Kivity <[email protected]>
Signed-off-by: Marcelo Tosatti <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
 arch/x86/kvm/vmx.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1470,13 +1470,6 @@ static void __vmx_load_host_state(struct
                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
@@ -6273,6 +6266,19 @@ static void __noclone vmx_vcpu_run(struc
 #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 linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to