From: Julien Grall Subject: 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 / XSA-385. Signed-off-by: Julien Grall Signed-off-by: Jan Beulich --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -2336,7 +2336,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); @@ -2345,8 +2346,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; --- 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++ ) {