[PATCH] xen/memory: Reject out-of-range resource 'frame' values

[Andrew Cooper <andrew.cooper3@citrix.com>]

The ABI is unfortunate, and frame being 64 bits leads to all kinds of problems
performing correct overflow checks.

Furthermore, the mixed use of unsigned int and unsigned long in the call tree
is buggy on arm32 where the two are the same size, and certain out-of-range
combinations will truncated and be permitted.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Paul Durrant <paul@xen.org>
CC: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Posted ahead of my full v8 series, in the hope that it catches some people
before the end of the day.
---
 xen/arch/x86/mm.c        |  2 +-
 xen/common/memory.c      | 15 ++++++++++++++-
 xen/include/asm-x86/mm.h |  2 +-
 xen/include/xen/mm.h     |  2 +-
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 63e9fae919..2ce00a8ece 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4588,7 +4588,7 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
 }
 
 int arch_acquire_resource(struct domain *d, unsigned int type,
-                          unsigned int id, unsigned long frame,
+                          unsigned int id, unsigned int frame,
                           unsigned int nr_frames, xen_pfn_t mfn_list[])
 {
     int rc;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index ccb4d49fc6..33ab36eb72 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1054,7 +1054,7 @@ static long xatp_permission_check(struct domain *d, unsigned int space)
 }
 
 static int acquire_grant_table(struct domain *d, unsigned int id,
-                               unsigned long frame,
+                               unsigned int frame,
                                unsigned int nr_frames,
                                xen_pfn_t mfn_list[])
 {
@@ -1134,6 +1134,19 @@ static int acquire_resource(
     if ( xmar.nr_frames > ARRAY_SIZE(mfn_list) )
         return -E2BIG;
 
+    /*
+     * The ABI is rather unfortunate.  nr_frames (and therefore the total size
+     * of the resource) is 32bit, while frame (the offset within the resource
+     * we'd like to start at) is 64bit.
+     *
+     * Reject values oustide the of the range of nr_frames, as well as
+     * combinations of frame and nr_frame which overflow, to simplify the rest
+     * of the logic.
+     */
+    if ( (xmar.frame >> 32) ||
+         ((xmar.frame + xmar.nr_frames) >> 32) )
+        return -EINVAL;
+
     rc = rcu_lock_remote_domain_by_id(xmar.domid, &d);
     if ( rc )
         return rc;
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 1fdb4eb835..3fe47dcd8d 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -641,7 +641,7 @@ static inline bool arch_mfn_in_directmap(unsigned long mfn)
 }
 
 int arch_acquire_resource(struct domain *d, unsigned int type,
-                          unsigned int id, unsigned long frame,
+                          unsigned int id, unsigned int frame,
                           unsigned int nr_frames, xen_pfn_t mfn_list[]);
 
 #endif /* __ASM_X86_MM_H__ */
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 636a1254ae..66a4d10695 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -614,7 +614,7 @@ static inline void put_page_alloc_ref(struct page_info *page)
 
 #ifndef CONFIG_ARCH_ACQUIRE_RESOURCE
 static inline int arch_acquire_resource(
-    struct domain *d, unsigned int type, unsigned int id, unsigned long frame,
+    struct domain *d, unsigned int type, unsigned int id, unsigned int frame,
     unsigned int nr_frames, xen_pfn_t mfn_list[])
 {
     return -EOPNOTSUPP;
-- 
2.11.0