openSUSE Build Service

Overview Repositories Revisions Requests Users Attributes Meta

File xsa262.patch of Package xen.7653

x86/HVM: guard against emulator driving ioreq state in weird ways

In the case where hvm_wait_for_io() calls wait_on_xen_event_channel(),
p->state ends up being read twice in succession: once to determine that
state != p->state, and then again at the top of the loop.  This gives a
compromised emulator a chance to change the state back between the two
reads, potentially keeping Xen in a loop indefinitely.

Instead:
* Read p->state once in each of the wait_on_xen_event_channel() tests,
* re-use that value the next time around,
* and insist that the states continue to transition "forward" (with the
  exception of the transition to STATE_IOREQ_NONE).

This is XSA-262.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -407,20 +407,31 @@ bool_t hvm_io_pending(struct vcpu *v)
 
 static bool_t hvm_wait_for_io(struct hvm_ioreq_vcpu *sv, ioreq_t *p)
 {
-    unsigned int state;
+    unsigned int state = p->state, prev_state = STATE_IOREQ_NONE;
 
-    /* NB. Optimised for common case (p->state == STATE_IOREQ_NONE). */
-    while ( (state = p->state) != STATE_IOREQ_NONE )
+    smp_rmb();
+
+    while ( likely(state != STATE_IOREQ_NONE) )
     {
-        rmb();
-        switch ( state )
+        if ( unlikely(state < prev_state) )
+        {
+            gdprintk(XENLOG_ERR, "Weird HVM ioreq state transition %u -> %u\n",
+                     prev_state, state);
+            domain_crash(sv->vcpu->domain);
+            return 0; /* bail */
+        }
+
+        switch ( prev_state = state )
         {
         case STATE_IORESP_READY: /* IORESP_READY -> NONE */
             hvm_io_assist(p);
-            break;
+            return 1;
         case STATE_IOREQ_READY:  /* IOREQ_{READY,INPROCESS} -> IORESP_READY */
         case STATE_IOREQ_INPROCESS:
-            wait_on_xen_event_channel(sv->ioreq_evtchn, p->state != state);
+            wait_on_xen_event_channel(sv->ioreq_evtchn,
+                                      ({ state = p->state;
+                                         smp_rmb();
+                                         state != prev_state; }));
             break;
         default:
             gdprintk(XENLOG_ERR, "Weird HVM iorequest state %u\n", state);

Places

File xsa262.patch of Package xen.7653

Places