xen/page_alloc: Harden assign_pages()

domain_tot_pages() and d->max_pages are 32-bit values. While the order
should always be quite small, it would still be possible to overflow
if domain_tot_pages() is near to (2^32 - 1).

As this code may be called by a guest via XENMEM_increase_reservation
and XENMEM_populate_physmap, we want to make sure the guest is not going
to be able to allocate more than it is allowed.

Rework the allocation check to avoid any possible overflow. While the
check domain_tot_pages() < d->max_pages should technically not be
necessary, it is probably best to have it to catch any possible
inconsistencies in the future.

This is CVE-2021-28706 / part of XSA-385.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 143501861d48e1bfef495849fd68584baac05849
master date: 2021-11-22 11:11:05 +0000

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index f8486c3de78c41eaaac74f249f7b68baad1cdafd..90781e236e17d2769f750d0266845150973b33e9 100644 (file)
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2319,7 +2319,8 @@ gnttab_transfer(
          * pages when it is dying.
          */
         if ( unlikely(e->is_dying) ||
-             unlikely(domain_tot_pages(e) >= e->max_pages) )
+             unlikely(domain_tot_pages(e) >= e->max_pages) ||
+             unlikely(!(e->tot_pages + 1)) )
         {
             spin_unlock(&e->page_alloc_lock);
 
@@ -2328,8 +2329,8 @@ gnttab_transfer(
                          e->domain_id);
             else
                 gdprintk(XENLOG_INFO,
-                         "Transferee d%d has no headroom (tot %u, max %u)\n",
-                         e->domain_id, domain_tot_pages(e), e->max_pages);
+                         "Transferee %pd has no headroom (tot %u, max %u, ex %u)\n",
+                         e, domain_tot_pages(e), e->max_pages, e->extra_pages);
 
             gop.status = GNTST_general_error;
             goto unlock_and_copyback;

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 8859fa4fd1ce2585fea187b61a9563ba36724734..4ec1b37ac9a14cf7cb01d9d37d80dc98788f8973 100644 (file)
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2298,20 +2298,43 @@ int assign_pages(
     }
     else if ( !(memflags & MEMF_no_refcount) )
     {
-        unsigned int tot_pages = domain_tot_pages(d) + (1 << order);
+        unsigned int tot_pages = domain_tot_pages(d), nr = 1u << order;
 
         if ( unlikely(tot_pages > d->max_pages) )
         {
-            gprintk(XENLOG_INFO, "Over-allocation for domain %u: "
-                    "%u > %u\n", d->domain_id, tot_pages, d->max_pages);
+            gprintk(XENLOG_INFO, "Inconsistent allocation for %pd: %u > %u\n",
+                    d, tot_pages, d->max_pages);
+            rc = -EPERM;
+            goto out;
+        }
+
+        if ( unlikely(nr > d->max_pages - tot_pages) )
+        {
+            gprintk(XENLOG_INFO, "Over-allocation for %pd: %Lu > %u\n",
+                    d, tot_pages + 0ull + nr, d->max_pages);
             rc = -E2BIG;
             goto out;
         }
     }
 
-    if ( !(memflags & MEMF_no_refcount) &&
-         unlikely(domain_adjust_tot_pages(d, 1 << order) == (1 << order)) )
-        get_knownalive_domain(d);
+    if ( !(memflags & MEMF_no_refcount) )
+    {
+        unsigned int nr = 1u << order;
+
+        if ( unlikely(d->tot_pages + nr < nr) )
+        {
+            gprintk(XENLOG_INFO,
+                    "Excess allocation for %pd: %Lu (%u extra)\n",
+                    d, d->tot_pages + 0ull + nr, d->extra_pages);
+            if ( pg[0].count_info & PGC_extra )
+                d->extra_pages -= nr;
+            rc = -E2BIG;
+            goto out;
+        }
+
+        if ( unlikely(domain_adjust_tot_pages(d, nr) == nr) )
+            get_knownalive_domain(d);
+    }
 
     for ( i = 0; i < (1 << order); i++ )
     {