[Xen-devel] [PATCH 2/2] events/fifo: don't corrupt queues if an old tail moves queues

[David Vrabel <david.vrabel@xxxxxxxxxx>]

From: David Vrabel <david.vrabel@xxxxxxxxxx>

An event may still be the tail of a queue even if the queue is now
empty (an 'old tail' event).  There is logic to handle the case when
this old tail event needs to be added to the now empty queue (by
checking for q->tail == port).

However, if the old tail event on queue A is moved to a different
queue B (by changing its VCPU or priority), the event may then be
linked onto queue B.  When another event is linked onto queue A it
will check the old tail, see that it is linked (but on queue B) and
overwrite the LINK field, corrupting both queues.

When an event is linked, save the vcpu id and priority of thee queue
it is being linked onto.  Use this when linking an event to check if
it is an unlinked old tail event. i.e., a) it has moved queues; b) it
is currently unlinked; and c) it's the tail of the old queue.  If it
is an unlinked, old tail event, the old queue is empty and old_q->tail
is invalidated to ensure adding another event to old_q will update
HEAD.  The tail is invalidated by setting it to 0 since the event 0 is
never linked.

The old_q->lock is held while setting LINKED to avoid a race with the
test of LINKED in evtchn_fifo_set_link().

Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
---
 xen/common/event_fifo.c |   42 +++++++++++++++++++++++++++++++++++++++++-
 xen/include/xen/sched.h |    2 ++
 2 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 7e9aa64..97b07f5 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -98,6 +98,46 @@ static bool_t evtchn_fifo_set_link(struct domain *d, 
event_word_t *word,
     return 1;
 }
 
+static bool_t test_and_set_linked(struct domain *d, struct evtchn *evtchn,
+                                  struct evtchn_fifo_queue *q,
+                                  event_word_t *word)
+{
+    struct vcpu *old_v;
+    struct evtchn_fifo_queue *old_q;
+    bool_t was_linked;
+    unsigned long flags;
+
+    old_v = d->vcpu[evtchn->last_vcpu_id];
+    old_q = &old_v->evtchn_fifo->queue[evtchn->last_priority];
+
+    evtchn->last_vcpu_id = evtchn->notify_vcpu_id;
+    evtchn->last_priority = evtchn->priority;
+
+    if ( q == old_q )
+        return test_and_set_bit(EVTCHN_FIFO_LINKED, word);
+
+    /*
+     * This event is now on a different queue.
+     *
+     * If the event is still linked in the old queue it won't be moved
+     * yet.
+     *
+     * If this event is unlinked /and/ it's the old queue's tail, the
+     * old queue is empty and its tail must be invalidated to prevent
+     * adding an event to the old queue from corrupting the new queue.
+     */
+    spin_lock_irqsave(&old_q->lock, flags);
+
+    was_linked = test_and_set_bit(EVTCHN_FIFO_LINKED, word);
+
+    if ( !was_linked && old_q->tail == evtchn->port )
+        old_q->tail = 0;
+
+    spin_unlock_irqrestore(&old_q->lock, flags);
+
+    return was_linked;
+}
+
 static void evtchn_fifo_set_pending(struct vcpu *v, struct evtchn *evtchn)
 {
     struct domain *d = v->domain;
@@ -133,7 +173,7 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
      * Link the event if it unmasked and not already linked.
      */
     if ( !test_bit(EVTCHN_FIFO_MASKED, word)
-         && !test_and_set_bit(EVTCHN_FIFO_LINKED, word) )
+         && !test_and_set_linked(d, evtchn, q, word) )
     {
         event_word_t *tail_word;
         bool_t linked = 0;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 2397537..3714c37 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -98,6 +98,8 @@ struct evtchn
     } u;
     u8 priority;
     u8 pending:1;
+    u16 last_vcpu_id;
+    u8 last_priority;
 #ifdef FLASK_ENABLE
     void *ssid;
 #endif
-- 
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel