XSA-73

CVE-2013-4494

问题描述

http://xenbits.xen.org/xsa/advisory-73.html

Lock order reversal between page allocation and grant table locks

The locks page_alloc_lock and grant_table.lock are not always taken in the same order. This opens the possibility of deadlock.

logic error (deadlock)

Patch描述

http://xenbits.xen.org/xsa/xsa73-4.2.patch

Correct a lock order reversal between a domains page allocation and grant table locks.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1499,6 +1499,8 @@ gnttab_transfer(
 
     for ( i = 0; i < count; i++ )
     {
+        bool_t okay;
+
         if (i && hypercall_preempt_check())
             return i;
 
@@ -1607,16 +1609,18 @@ gnttab_transfer(
          * pages when it is dying.
          */
         if ( unlikely(e->is_dying) ||
-             unlikely(e->tot_pages >= e->max_pages) ||
-             unlikely(!gnttab_prepare_for_transfer(e, d, gop.ref)) )
+             unlikely(e->tot_pages >= e->max_pages) )
         {
-            if ( !e->is_dying )
-                gdprintk(XENLOG_INFO, "gnttab_transfer: "
-                        "Transferee has no reservation "
-                        "headroom (%d,%d) or provided a bad grant ref (%08x) "
-                        "or is dying (%d)\n",
-                        e->tot_pages, e->max_pages, gop.ref, e->is_dying);
             spin_unlock(&e->page_alloc_lock);
+
+            if ( e->is_dying )
+                gdprintk(XENLOG_INFO, "gnttab_transfer: "
+                         "Transferee (d%d) is dying\n", e->domain_id);
+            else
+                gdprintk(XENLOG_INFO, "gnttab_transfer: "
+                         "Transferee (d%d) has no headroom (tot %u, max %u)\n",
+                         e->domain_id, e->tot_pages, e->max_pages);
+
             rcu_unlock_domain(e);
             put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
@@ -1628,6 +1632,38 @@ gnttab_transfer(
         /* Okay, add the page to 'e'. */
         if ( unlikely(e->tot_pages++ == 0) )
             get_knownalive_domain(e);
+
+        /*
+         * We must drop the lock to avoid a possible deadlock in
+         * gnttab_prepare_for_transfer.  We have reserved a page in e so can
+         * safely drop the lock and re-aquire it later to add page to the
+         * pagelist.
+         */
+        spin_unlock(&e->page_alloc_lock);
+        okay = gnttab_prepare_for_transfer(e, d, gop.ref);
+        spin_lock(&e->page_alloc_lock);
+
+        if ( unlikely(!okay) || unlikely(e->is_dying) )
+        {
+            bool_t drop_dom_ref = (e->tot_pages-- == 1);
+
+            spin_unlock(&e->page_alloc_lock);
+
+            if ( okay /* i.e. e->is_dying due to the surrounding if() */ )
+                gdprintk(XENLOG_INFO, "gnttab_transfer: "
+                         "Transferee (d%d) is now dying\n", e->domain_id);
+
+            if ( drop_dom_ref )
+                put_domain(e);
+            rcu_unlock_domain(e);
+
+            put_gfn(d, gop.mfn);
+            page->count_info &= ~(PGC_count_mask|PGC_allocated);
+            free_domheap_page(page);
+            gop.status = GNTST_general_error;
+            goto copyback;
+        }
+
         page_list_add_tail(page, &e->page_list);
         page_set_owner(page, e);

Consequence

A malicious guest administrator can deny service to the entire host.

DoS