[xen master] memory: XENMEM_add_to_physmap (almost) wrapping checks

[patchbot@xxxxxxx]

commit ef0f94a48fd7b6a33b4460e545572c65573cc3e0
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Feb 2 10:26:06 2022 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Feb 2 10:26:06 2022 +0100

    memory: XENMEM_add_to_physmap (almost) wrapping checks
    
    Determining that behavior is correct (i.e. results in failure) for a
    passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit
    more obvious by checking input in generic code - both for singular
    requests to not match the value and for range ones to not pass / wrap
    through it.
    
    For Arm similarly make more obvious that no wrapping of MFNs passed
    for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur:
    Drop the "nr" parameter of the function to avoid future callers
    appearing which might not themselves check for wrapping. Otherwise
    the respective ASSERT() in rangeset_contains_range() could trigger.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 xen/arch/arm/include/asm/p2m.h |  5 +----
 xen/arch/arm/mm.c              |  2 +-
 xen/arch/arm/p2m.c             | 13 +++++--------
 xen/common/grant_table.c       |  3 +++
 xen/common/memory.c            | 18 ++++++++++++++++++
 5 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
index 8f11d9c97b..8cce459b67 100644
--- a/xen/arch/arm/include/asm/p2m.h
+++ b/xen/arch/arm/include/asm/p2m.h
@@ -295,10 +295,7 @@ int unmap_regions_p2mt(struct domain *d,
                        unsigned long nr,
                        mfn_t mfn);
 
-int map_dev_mmio_region(struct domain *d,
-                        gfn_t gfn,
-                        unsigned long nr,
-                        mfn_t mfn);
+int map_dev_mmio_page(struct domain *d, gfn_t gfn, mfn_t mfn);
 
 int p2m_insert_mapping(struct domain *d, gfn_t start_gfn, unsigned long nr,
                        mfn_t mfn, p2m_type_t t);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index eea926d823..b1eae767c2 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one(
         break;
     }
     case XENMAPSPACE_dev_mmio:
-        rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx));
+        rc = map_dev_mmio_page(d, gfn, _mfn(idx));
         return rc;
 
     default:
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index fb71fa4c1c..02cf852d4c 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1355,21 +1355,18 @@ int unmap_mmio_regions(struct domain *d,
     return p2m_remove_mapping(d, start_gfn, nr, mfn);
 }
 
-int map_dev_mmio_region(struct domain *d,
-                        gfn_t gfn,
-                        unsigned long nr,
-                        mfn_t mfn)
+int map_dev_mmio_page(struct domain *d, gfn_t gfn, mfn_t mfn)
 {
     int res;
 
-    if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) )
+    if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) )
         return 0;
 
-    res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c);
+    res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c);
     if ( res < 0 )
     {
-        printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in 
Dom%d\n",
-               mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id);
+        printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n",
+               mfn_x(mfn), d);
         return res;
     }
 
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index ed1e2fabce..233c4bfcbe 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4157,7 +4157,10 @@ int gnttab_map_frame(struct domain *d, unsigned long 
idx, gfn_t gfn, mfn_t *mfn)
     bool status = false;
 
     if ( gfn_eq(gfn, INVALID_GFN) )
+    {
+        ASSERT_UNREACHABLE();
         return -EINVAL;
+    }
 
     grant_write_lock(gt);
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 30d255da35..0d7c413df8 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -832,6 +832,9 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
         return -EACCES;
     }
 
+    if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) )
+        return -EINVAL;
+
     if ( xatp->space == XENMAPSPACE_gmfn_foreign )
         extra.foreign_domid = DOMID_INVALID;
 
@@ -842,6 +845,18 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
     if ( xatp->size < start )
         return -EILSEQ;
 
+    if ( xatp->gpfn + xatp->size < xatp->gpfn ||
+         xatp->idx + xatp->size < xatp->idx )
+    {
+        /*
+         * Make sure INVALID_GFN is the highest representable value, i.e.
+         * guaranteeing that it won't fall in the middle of the
+         * [xatp->gpfn, xatp->gpfn + xatp->size) range checked above.
+         */
+        BUILD_BUG_ON(INVALID_GFN_RAW + 1);
+        return -EOVERFLOW;
+    }
+
     xatp->idx += start;
     xatp->gpfn += start;
     xatp->size -= start;
@@ -962,6 +977,9 @@ static int xenmem_add_to_physmap_batch(struct domain *d,
                                                extent, 1)) )
             return -EFAULT;
 
+        if ( gfn_eq(_gfn(gpfn), INVALID_GFN) )
+            return -EINVAL;
+
         rc = xenmem_add_to_physmap_one(d, xatpb->space, extra,
                                        idx, _gfn(gpfn));
 
--
generated by git-patchbot for /home/xen/git/xen.git#master