[PATCH 2/2] xen/mm: do not assign pages to a domain until they are scrubbed

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

Assigning pages to a domain make them the possible target of hypercalls
like XENMEM_decrease_reservation ahead of such pages being scrubbed in
populate_physmap() when the guest is running in PV mode.  This might allow
pages to be freed ahead of being scrubbed for example, as a stubdomain
already running could target them by guessing their MFNs.  It's also
possible other action could set the page type ahead of scrubbing, which
would be problematic.

Prevent the pages pending scrub from being assigned to the domain, and only
do the assign once the scrubbing has finished.  This has the disadvantage
that the allocated pages will be removed from the free pool, but not yet
accounted towards the domain consumed page quota.  However there can only
be one stashed page in that state, and it's maximum size is bounded by the
memop-max-order option.  This is not too different from the current logic,
where assigning pages to a domain (and thus checking whether such domain
doesn't overflow it's quota) is also done after the memory has been
allocated and removed from the pool of free pages.

Fixes: 83a784a15b47 ("xen/mm: allow deferred scrub of physmap populate 
allocated pages")
Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
I've attempted various different ways to solve this, but they all ended up
being impossible.

 * Prevent non-scrubbed pages from getting extra refcounts (iow: make
   get_page() fail for them).  This seemed nice, but the cleanup using
   put_page_alloc_ref() was impossible as non-scrubbed pages would return
   failure in get_page(), and so I couldn't take the extra reference ahead
   of calling put_page_alloc_ref().
 * Disallow XENMEM_decrease_reservation until the domain has finished
   creation would fix the issue of pages being freed while pending scrub,
   but it's not clear there might be other usages that would be problematic,
   as get_page() on non-scrubbed pages would still return success.
---
 xen/common/memory.c     | 6 ++++++
 xen/common/page_alloc.c | 8 +++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index f0ff1311881c..1ad4b51c5b02 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -388,6 +388,12 @@ static void populate_physmap(struct memop_args *a)
                             goto out;
                         }
                     }
+
+                    if ( assign_page(page, a->extent_order, d, memflags) )
+                    {
+                        free_domheap_pages(page, a->extent_order);
+                        goto out;
+                    }
                 }
 
                 if ( unlikely(a->memflags & MEMF_no_tlbflush) )
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 1316dfbd15ee..b72a74c705ba 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2699,7 +2699,13 @@ struct page_info *alloc_domheap_pages(
                                   memflags, d)) == NULL)) )
          return NULL;
 
-    if ( d && !(memflags & MEMF_no_owner) )
+    /*
+     * Don't add pages with the PGC_need_scrub bit set to the domain, the
+     * caller must clean the bit and then manually call assign_pages().
+     * Otherwise pages with the PGC_need_scrub would be reachable using
+     * get_page().
+     */
+    if ( d && !(memflags & MEMF_no_owner) && !(memflags & MEMF_keep_scrub) )
     {
         if ( memflags & MEMF_no_refcount )
         {
-- 
2.51.0