[xen/f19] Lock order reversal between page allocation and grant table locks

myoung myoung at fedoraproject.org
Sat Nov 2 00:02:03 UTC 2013


commit e28a2892abdaab4ef4e050d142222adf31ed7cbf
Author: Michael Young <m.a.young at durham.ac.uk>
Date:   Sat Nov 2 00:01:56 2013 +0000

    Lock order reversal between page allocation and grant table locks

 xen.spec        |    8 ++++-
 xsa73-4.2.patch |  108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+), 1 deletions(-)
---
diff --git a/xen.spec b/xen.spec
index 5f1cb0b..0fe8bec 100644
--- a/xen.spec
+++ b/xen.spec
@@ -27,7 +27,7 @@
 Summary: Xen is a virtual machine monitor
 Name:    xen
 Version: 4.2.3
-Release: 6%{?dist}
+Release: 7%{?dist}
 Group:   Development/Libraries
 License: GPLv2+ and LGPLv2+ and BSD
 URL:     http://xen.org/
@@ -90,6 +90,7 @@ Patch112: xsa69.patch
 Patch113: xsa70.patch
 Patch114: xsa71-qemu-xen-4.2.patch
 Patch115: xsa72.patch
+Patch116: xsa73-4.2.patch
 
 Patch100: xen-configure-xend.patch
 
@@ -270,6 +271,7 @@ manage Xen virtual machines.
 %patch113 -p1
 %patch114 -p1
 %patch115 -p1
+%patch116 -p1
 
 %patch100 -p1
 
@@ -763,6 +765,10 @@ rm -rf %{buildroot}
 %endif
 
 %changelog
+* Fri Nov 01 2013 Michael Young <m.a.young at durham.ac.uk> - 4.2.3-7
+- Lock order reversal between page allocation and grant table locks
+    [XSA-73, CVE-2013-4494]
+
 * Tue Oct 29 2013 Michael Young <m.a.young at durham.ac.uk> - 4.2.3-6
 - ocaml xenstored mishandles oversized message replies
     [XSA-72, CVE-2013-4416] (#1024450)
diff --git a/xsa73-4.2.patch b/xsa73-4.2.patch
new file mode 100644
index 0000000..b4bb398
--- /dev/null
+++ b/xsa73-4.2.patch
@@ -0,0 +1,108 @@
+From 52b2c3148bdcaa46befcdca64e14d0201d7ca642 Mon Sep 17 00:00:00 2001
+From: Andrew Cooper <andrew.cooper3 at citrix.com>
+Date: Thu, 31 Oct 2013 20:49:00 +0000
+Subject: [PATCH] gnttab: correct locking order reversal
+
+Coverity ID 1087189
+
+Correct a lock order reversal between a domains page allocation and grant
+table locks.
+
+This is XSA-73.
+
+Signed-off-by: Andrew Cooper <andrew.cooper3 at citrix.com>
+
+Consolidate error handling.
+
+Signed-off-by: Jan Beulich <jbeulich at suse.com>
+Reviewed-by: Keir Fraser <keir at xen.org>
+Tested-by: Matthew Daley <mattjd at gmail.com>
+
+Backported to Xen-4.2
+Signed-off-by: Andrew Cooper <andrew.cooper3 at citrix.com>
+---
+ xen/common/grant_table.c |   52 +++++++++++++++++++++++++++++++++++++++-------
+ 1 file changed, 44 insertions(+), 8 deletions(-)
+
+diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
+index 0e349cc..0672bad 100644
+--- 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);
+ 
+-- 
+1.7.10.4
+


More information about the scm-commits mailing list