[Xen-devel] [PATCH] gnttab: fix page_alloc + grant_table deadlock

[Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>]

AB/BA deaclock between d->page_alloc_lock and d->grant_table->lock
happened in this scenario: (names are original)

 gnttab_transfer():               | gnttab_setup_table():
  spin_lock(&e->page_alloc_lock); +  spin_lock(&gt->lock);
  gnttab_prepare_for_transfer():  |  gnttab_grow_table():
                                  |   gnttab_create_shared_page():
                                  |    share_xen_page_with_guest():
    spin_lock(&rgt->lock);        +      spin_lock(&d->page_alloc_lock);

The patch takes grant_table->lock first in gnttab_transfer() and keeps
it longer, which also saves one unlock()+lock().

Based on analysis from Ulrich Obergfell <uobergfe@xxxxxxxxxx>.

Signed-off-by: Radim KrÄmÃÅ <rkrcmar@xxxxxxxxxx>
---
 xen/common/grant_table.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index f42bc7a..fa46a8b 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1434,6 +1434,7 @@ gnttab_query_size(
 /*
  * Check that the given grant reference (rd,ref) allows 'ld' to transfer
  * ownership of a page frame. If so, lock down the grant entry.
+ * rd's grant table lock must be held by the caller.
  */
 static int 
 gnttab_prepare_for_transfer(
@@ -1444,8 +1445,6 @@ gnttab_prepare_for_transfer(
     union grant_combo   scombo, prev_scombo, new_scombo;
     int                 retries = 0;
 
-    spin_lock(&rgt->lock);
-
     if ( rgt->gt_version == 0 )
     {
         gdprintk(XENLOG_INFO,
@@ -1495,11 +1494,9 @@ gnttab_prepare_for_transfer(
         scombo = prev_scombo;
     }
 
-    spin_unlock(&rgt->lock);
     return 1;
 
  fail:
-    spin_unlock(&rgt->lock);
     return 0;
 }
 
@@ -1617,6 +1614,7 @@ gnttab_transfer(
             page = new_page;
         }
 
+        spin_lock(&e->grant_table->lock);
         spin_lock(&e->page_alloc_lock);
 
         /*
@@ -1635,6 +1633,7 @@ gnttab_transfer(
                         "or is dying (%d)\n",
                         e->tot_pages, e->max_pages, gop.ref, e->is_dying);
             spin_unlock(&e->page_alloc_lock);
+            spin_unlock(&e->grant_table->lock);
             rcu_unlock_domain(e);
             put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
@@ -1655,8 +1654,6 @@ gnttab_transfer(
         TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id);
 
         /* Tell the guest about its new page frame. */
-        spin_lock(&e->grant_table->lock);
-
         if ( e->grant_table->gt_version == 1 )
         {
             grant_entry_v1_t *sha = &shared_entry_v1(e->grant_table, gop.ref);
-- 
1.8.4.2


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