]> xenbits.xensource.com Git - xen.git/commitdiff
x86/HVM: avoid reading ioreq state more than once
authorJan Beulich <jbeulich@suse.com>
Thu, 17 Dec 2015 13:33:22 +0000 (14:33 +0100)
committerJan Beulich <jbeulich@suse.com>
Thu, 17 Dec 2015 13:33:22 +0000 (14:33 +0100)
Otherwise, especially when the compiler chooses to translate the
switch() to a jump table, unpredictable behavior (and in the jump table
case arbitrary code execution) can result.

This is XSA-166.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
master commit: b452430a4cdfc801fa4bc391aed7522365e1deb6
master date: 2015-12-17 14:22:46 +0100

xen/arch/x86/hvm/hvm.c

index c731cc46091ab251157510cec63bf9a0b4f1f2c0..9dafccaaabb0a464ba64101dfda9904be1875ad5 100644 (file)
@@ -342,6 +342,7 @@ void hvm_migrate_pirqs(struct vcpu *v)
 void hvm_do_resume(struct vcpu *v)
 {
     ioreq_t *p;
+    unsigned int state;
 
     pt_restore_timer(v);
 
@@ -349,9 +350,10 @@ void hvm_do_resume(struct vcpu *v)
 
     /* NB. Optimised for common case (p->state == STATE_IOREQ_NONE). */
     p = get_ioreq(v);
-    while ( p->state != STATE_IOREQ_NONE )
+    while ( (state = p->state) != STATE_IOREQ_NONE )
     {
-        switch ( p->state )
+        rmb();
+        switch ( state )
         {
         case STATE_IORESP_READY: /* IORESP_READY -> NONE */
             hvm_io_assist();
@@ -359,11 +361,10 @@ void hvm_do_resume(struct vcpu *v)
         case STATE_IOREQ_READY:  /* IOREQ_{READY,INPROCESS} -> IORESP_READY */
         case STATE_IOREQ_INPROCESS:
             wait_on_xen_event_channel(v->arch.hvm_vcpu.xen_port,
-                                      (p->state != STATE_IOREQ_READY) &&
-                                      (p->state != STATE_IOREQ_INPROCESS));
+                                      p->state != state);
             break;
         default:
-            gdprintk(XENLOG_ERR, "Weird HVM iorequest state %d.\n", p->state);
+            gdprintk(XENLOG_ERR, "Weird HVM iorequest state %u\n", state);
             domain_crash(v->domain);
             return; /* bail */
         }