[PATCH v2] memory: check incoming GFNs

[Jan Beulich <jbeulich@xxxxxxxx>]

P2M handling (for translated domains) is at best undefined when passing in
GFNs which aren't aligned according to the requested order.

For non-translated domains the same isn't necessarily true, and domains
may possibly be able to leverage present behavior; in fact
XENMEM_memory_exchange is known to be used in such ways. Instead the
supplied GPFN has to at least pass VALID_M2P().

In memory_exchange() go farther than merely adding checks: Restrict the
scopes of the two variables such that it becomes more clear across which
range of code the validity checks actually apply. Also don't limit
checking there to non-translated domains (as of now steal_page() would
prevent things from working on translated ones), to avoid giving bad
examples or leaving latent traps.

Furthermore, covering translated domains in memory_exchange() is forward-
looking: Right now steal_page() prevents (successful) use of that sub-op
by such domains, but there are intentions to relax that.

Fixes: e4dfe0cdba5e ("Replace dom_mem_op hypercall with memory_op hypercall 
with")
Fixes: 6f8668f57256 ("New memory hypercall 'populate_physmap'")
Fixes: 516250dac6a8 ("New memory_op XENMEM_exchange")
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I'm not sure at all whether for the direct-mapped case the checking in
populate_physmap() is appropriate.
---
v2: Drop the paging_mode_translate() part from the two GMFN checks.
    Tighten the GPFN check in populate_physmap(). Add ChangeLog entry.

--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog
 ### Changed
  - XEN_DOMCTL_DEV_DT's, FLASK_[GS]ETBOOL's, and FLASK_DEVICETREE_LABEL's input
    string sizes need to include the nul terminator.
+ - XEMMEM_populate_physmap, XENMEM_decrease_reservation, and
+   XENMEM_memory_exchange now check input frame numbers more strictly (for
+   alignment according to the indicated order).
 
 ### Added
 
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -40,6 +40,10 @@
 #include <asm/guest.h>
 #endif
 
+#ifndef VALID_M2P
+# define VALID_M2P(gpfn) ((gpfn) != INVALID_M2P_ENTRY)
+#endif
+
 struct memop_args {
     /* INPUT */
     struct domain *domain;     /* Domain to be affected. */
@@ -277,6 +281,10 @@ static void populate_physmap(struct memo
         if ( unlikely(__copy_from_guest_offset(&gpfn, a->extent_list, i, 1)) )
             goto out;
 
+        if ( !IS_ALIGNED(gpfn, 1UL << a->extent_order) ||
+             (!paging_mode_translate(d) && !VALID_M2P(gpfn)) )
+            goto out;
+
         if ( a->memflags & MEMF_populate_on_demand )
         {
             /* Disallow populating PoD pages on oneself. */
@@ -586,6 +594,9 @@ static void decrease_reservation(struct
         if ( unlikely(__copy_from_guest_offset(&gmfn, a->extent_list, i, 1)) )
             goto out;
 
+        if ( !IS_ALIGNED(gmfn, 1UL << a->extent_order) )
+            goto out;
+
         if ( tb_init_done )
         {
             struct {
@@ -665,7 +676,6 @@ static long memory_exchange(XEN_GUEST_HA
     PAGE_LIST_HEAD(in_chunk_list);
     PAGE_LIST_HEAD(out_chunk_list);
     unsigned long in_chunk_order, out_chunk_order;
-    xen_pfn_t     gpfn, gmfn;
     mfn_t         mfn;
     unsigned long i, j, k;
     unsigned int  memflags = 0;
@@ -778,6 +788,8 @@ static long memory_exchange(XEN_GUEST_HA
         /* Steal a chunk's worth of input pages from the domain. */
         for ( j = 0; j < (1UL << in_chunk_order); j++ )
         {
+            xen_pfn_t gmfn;
+
             if ( unlikely(__copy_from_guest_offset(
                 &gmfn, exch.in.extent_start, (i<<in_chunk_order)+j, 1)) )
             {
@@ -785,6 +797,12 @@ static long memory_exchange(XEN_GUEST_HA
                 goto fail;
             }
 
+            if ( !IS_ALIGNED(gmfn, 1UL << exch.in.extent_order) )
+            {
+                rc = -EDOM;
+                goto fail;
+            }
+
             for ( k = 0; k < (1UL << exch.in.extent_order); k++ )
             {
 #ifdef CONFIG_X86
@@ -867,6 +885,8 @@ static long memory_exchange(XEN_GUEST_HA
         /* Assign each output page to the domain. */
         for ( j = 0; (page = page_list_remove_head(&out_chunk_list)); ++j )
         {
+            xen_pfn_t gpfn;
+
             if ( assign_page(page, exch.out.extent_order, d,
                              MEMF_no_refcount) )
             {
@@ -903,6 +923,18 @@ static long memory_exchange(XEN_GUEST_HA
                 continue;
             }
 
+            /*
+             * PV domains are known to pass arbitrary PFNs into this hypercall.
+             * Therefore we cannot apply the alignment check there.
+             */
+            if ( paging_mode_translate(d)
+                 ? !IS_ALIGNED(gpfn, 1UL << exch.out.extent_order)
+                 : !VALID_M2P(gpfn) )
+            {
+                rc = -EDOM;
+                continue;
+            }
+
             mfn = page_to_mfn(page);
             rc = guest_physmap_add_page(d, _gfn(gpfn), mfn,
                                         exch.out.extent_order) ?: rc;