[Xen-changelog] [xen master] evtchn: check control block exists when using FIFO-based events

[patchbot@xxxxxxx]

commit a4e0cea6fced50e251453dfe52e1b9dde77a84f5
Author:     David Vrabel <david.vrabel@xxxxxxxxxx>
AuthorDate: Tue Sep 9 15:25:58 2014 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Sep 9 15:25:58 2014 +0200

    evtchn: check control block exists when using FIFO-based events
    
    When using the FIFO-based event channels, there are no checks for the
    existance of a control block when binding an event or moving it to a
    different VCPU.  This is because events may be bound when the ABI is
    in 2-level mode (e.g., by the toolstack before the domain is started).
    
    The guest may trigger a Xen crash in evtchn_fifo_set_pending() if:
    
      a) the event is bound to a VCPU without a control block; or
      b) VCPU 0 does not have a control block.
    
    In case (a), Xen will crash when looking up the current queue.  In
    (b), Xen will crash when looking up the old queue (which defaults to a
    queue on VCPU 0).
    
    By allocating all the per-VCPU structures when enabling the FIFO ABI,
    we can be sure that v->evtchn_fifo is always valid.
    
    EVTCHNOP_init_control_block for all the other CPUs need only map the
    shared control block.
    
    A single check in evtchn_fifo_set_pending() before accessing the
    control block fixes all cases where the guest has not initialized some
    control blocks.
    
    This is XSA-107.
    
    Reported-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
    Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/common/event_fifo.c |   81 +++++++++++++++++++++++++++++++++--------------
 1 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 51b4ff6..b81fae4 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -178,6 +178,18 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
         bool_t linked = 0;
 
         /*
+         * Control block not mapped.  The guest must not unmask an
+         * event until the control block is initialized, so we can
+         * just drop the event.
+         */
+        if ( unlikely(!v->evtchn_fifo->control_block) )
+        {
+            printk(XENLOG_G_WARNING
+                   "%pv has no FIFO event channel control block\n", v);
+            goto done;
+        }
+
+        /*
          * No locking around getting the queue. This may race with
          * changing the priority but we are allowed to signal the
          * event once on the old priority.
@@ -385,36 +397,42 @@ static void init_queue(struct vcpu *v, struct 
evtchn_fifo_queue *q,
 {
     spin_lock_init(&q->lock);
     q->priority = i;
-    q->head = &v->evtchn_fifo->control_block->head[i];
 }
 
-static int setup_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset)
+static int setup_control_block(struct vcpu *v)
 {
-    struct domain *d = v->domain;
     struct evtchn_fifo_vcpu *efv;
-    void *virt;
     unsigned int i;
-    int rc;
-
-    if ( v->evtchn_fifo )
-        return -EINVAL;
 
     efv = xzalloc(struct evtchn_fifo_vcpu);
     if ( !efv )
         return -ENOMEM;
 
-    rc = map_guest_page(d, gfn, &virt);
+    for ( i = 0; i <= EVTCHN_FIFO_PRIORITY_MIN; i++ )
+        init_queue(v, &efv->queue[i], i);
+
+    v->evtchn_fifo = efv;
+
+    return 0;
+}
+
+static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset)
+{
+    void *virt;
+    unsigned int i;
+    int rc;
+
+    if ( v->evtchn_fifo->control_block )
+        return -EINVAL;
+
+    rc = map_guest_page(v->domain, gfn, &virt);
     if ( rc < 0 )
-    {
-        xfree(efv);
         return rc;
-    }
 
-    v->evtchn_fifo = efv;
     v->evtchn_fifo->control_block = virt + offset;
 
     for ( i = 0; i <= EVTCHN_FIFO_PRIORITY_MIN; i++ )
-        init_queue(v, &v->evtchn_fifo->queue[i], i);
+        v->evtchn_fifo->queue[i].head = 
&v->evtchn_fifo->control_block->head[i];
 
     return 0;
 }
@@ -509,28 +527,43 @@ int evtchn_fifo_init_control(struct evtchn_init_control 
*init_control)
 
     spin_lock(&d->event_lock);
 
-    rc = setup_control_block(v, gfn, offset);
-
     /*
      * If this is the first control block, setup an empty event array
      * and switch to the fifo port ops.
      */
-    if ( rc == 0 && !d->evtchn_fifo )
+    if ( !d->evtchn_fifo )
     {
+        struct vcpu *vcb;
+
+        for_each_vcpu ( d, vcb ) {
+            rc = setup_control_block(vcb);
+            if ( rc < 0 )
+                goto error;
+        }
+
         rc = setup_event_array(d);
         if ( rc < 0 )
-            cleanup_control_block(v);
-        else
-        {
-            d->evtchn_port_ops = &evtchn_port_ops_fifo;
-            d->max_evtchns = EVTCHN_FIFO_NR_CHANNELS;
-            setup_ports(d);
-        }
+            goto error;
+
+        rc = map_control_block(v, gfn, offset);
+        if ( rc < 0 )
+            goto error;
+
+        d->evtchn_port_ops = &evtchn_port_ops_fifo;
+        d->max_evtchns = EVTCHN_FIFO_NR_CHANNELS;
+        setup_ports(d);
     }
+    else
+        rc = map_control_block(v, gfn, offset);
 
     spin_unlock(&d->event_lock);
 
     return rc;
+
+ error:
+    evtchn_fifo_destroy(d);
+    spin_unlock(&d->event_lock);
+    return rc;
 }
 
 static int add_page_to_event_array(struct domain *d, unsigned long gfn)
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog