[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen master] evtchn/fifo: don't spin indefinitely when setting LINK



commit dae2a1c5f9372be1b5d1e734ad3e51f67a1221aa
Author:     David Vrabel <david.vrabel@xxxxxxxxxx>
AuthorDate: Tue Nov 12 13:19:25 2013 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Nov 12 13:19:25 2013 +0100

    evtchn/fifo: don't spin indefinitely when setting LINK
    
    A malicious or buggy guest can cause another domain to spin
    indefinitely by repeatedly writing to an event word when the other
    guest is trying to link a new event.  The cmpxchg() in
    evtchn_fifo_set_link() will repeatedly fail and the loop may never
    terminate.
    
    Fixing this requires a change to the ABI which is documented in draft
    H of the design.
    
      http://xenbits.xen.org/people/dvrabel/event-channels-H.pdf
    
    Since a well-behaved guest only makes a limited set of state changes,
    the loop can terminate early if the guest makes an invalid state
    transition.
    
    The guest may:
    
    - clear LINKED and LINK.
    - clear PENDING
    - set MASKED
    - clear MASKED
    
    It is valid for the guest to mask and unmask an event at any time so
    specify that it is not valid for a guest to clear MASKED if Xen is
    trying to update LINK.  Indicate this to the guest with an additional
    BUSY bit in the event word.  The guest must not clear MASKED if BUSY
    is set and it should spin until BUSY is cleared.
    
    The remaining valid writes (clear LINKED, clear PENDING, set MASKED,
    clear MASKED by Xen) will limit the number of failures of the
    cmpxchg() to at most 4.  A clear of LINKED will also terminate the
    loop early. Therefore, the loop can then be limited to at most 4
    iterations.
    
    If the buggy or malicious guest does cause the loop to exit with
    LINKED set and LINK unset then that buggy guest will lose events.
    
    Reported-by: Anthony Liguori <aliguori@xxxxxxxxxx>
    Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/common/event_fifo.c            |   75 +++++++++++++++++++++++++++++------
 xen/include/public/event_channel.h |    1 +
 2 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index 64dbfff..9106c55 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -34,19 +34,67 @@ static inline event_word_t 
*evtchn_fifo_word_from_port(struct domain *d,
     return d->evtchn_fifo->event_array[p] + w;
 }
 
-static bool_t evtchn_fifo_set_link(event_word_t *word, uint32_t link)
+static int try_set_link(event_word_t *word, event_word_t *w, uint32_t link)
 {
-    event_word_t n, o, w;
+    event_word_t new, old;
 
-    w = *word;
+    if ( !(*w & (1 << EVTCHN_FIFO_LINKED)) )
+        return 0;
 
-    do {
-        if ( !(w & (1 << EVTCHN_FIFO_LINKED)) )
-            return 0;
-        o = w;
-        n = (w & ~EVTCHN_FIFO_LINK_MASK) | link;
-    } while ( (w = cmpxchg(word, o, n)) != o );
+    old = *w;
+    new = (old & ~((1 << EVTCHN_FIFO_BUSY) | EVTCHN_FIFO_LINK_MASK)) | link;
+    *w = cmpxchg(word, old, new);
+    if ( *w == old )
+        return 1;
 
+    return -EAGAIN;
+}
+
+/*
+ * Atomically set the LINK field iff it is still LINKED.
+ *
+ * The guest is only permitted to make the following changes to a
+ * LINKED event.
+ *
+ * - set MASKED
+ * - clear MASKED
+ * - clear PENDING
+ * - clear LINKED (and LINK)
+ *
+ * We block unmasking by the guest by marking the tail word as BUSY,
+ * therefore, the cmpxchg() may fail at most 4 times.
+ */
+static bool_t evtchn_fifo_set_link(const struct domain *d, event_word_t *word,
+                                   uint32_t link)
+{
+    event_word_t w;
+    unsigned int try;
+    int ret;
+
+    w = read_atomic(word);
+
+    ret = try_set_link(word, &w, link);
+    if ( ret >= 0 )
+        return ret;
+
+    /* Lock the word to prevent guest unmasking. */
+    set_bit(EVTCHN_FIFO_BUSY, word);
+
+    w = read_atomic(word);
+
+    for ( try = 0; try < 4; try++ )
+    {
+        ret = try_set_link(word, &w, link);
+        if ( ret >= 0 )
+        {
+            if ( ret == 0 )
+                clear_bit(EVTCHN_FIFO_BUSY, word);
+            return ret;
+        }
+    }
+    gdprintk(XENLOG_WARNING, "domain %d, port %d not linked\n",
+             d->domain_id, link);
+    clear_bit(EVTCHN_FIFO_BUSY, word);
     return 1;
 }
 
@@ -105,7 +153,7 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct 
evtchn *evtchn)
         if ( port != q->tail )
         {
             tail_word = evtchn_fifo_word_from_port(d, q->tail);
-            linked = evtchn_fifo_set_link(tail_word, port);
+            linked = evtchn_fifo_set_link(d, tail_word, port);
         }
         if ( !linked )
             write_atomic(q->head, port);
@@ -202,11 +250,12 @@ static void evtchn_fifo_print_state(struct domain *d,
 
     word = evtchn_fifo_word_from_port(d, evtchn->port);
     if ( !word )
-        printk("?   ");
+        printk("?     ");
     else if ( test_bit(EVTCHN_FIFO_LINKED, word) )
-        printk("%-4u", *word & EVTCHN_FIFO_LINK_MASK);
+        printk("%c %-4u", test_bit(EVTCHN_FIFO_BUSY, word) ? 'B' : ' ',
+               *word & EVTCHN_FIFO_LINK_MASK);
     else
-        printk("-   ");
+        printk("%c -   ", test_bit(EVTCHN_FIFO_BUSY, word) ? 'B' : ' ');
 }
 
 static const struct evtchn_port_ops evtchn_port_ops_fifo =
diff --git a/xen/include/public/event_channel.h 
b/xen/include/public/event_channel.h
index 4a53484..b78ee32 100644
--- a/xen/include/public/event_channel.h
+++ b/xen/include/public/event_channel.h
@@ -343,6 +343,7 @@ typedef uint32_t event_word_t;
 #define EVTCHN_FIFO_PENDING 31
 #define EVTCHN_FIFO_MASKED  30
 #define EVTCHN_FIFO_LINKED  29
+#define EVTCHN_FIFO_BUSY    28
 
 #define EVTCHN_FIFO_LINK_BITS 17
 #define EVTCHN_FIFO_LINK_MASK ((1 << EVTCHN_FIFO_LINK_BITS) - 1)
--
generated by git-patchbot for /home/xen/git/xen.git#master

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.