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

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

Reject out-of-range values, and combinations which overflow, and use unsigned
int consistently elsewhere.  This fixes several truncation bugs in the grant
call tree, as the underlying limits are expressed with unsigned int to begin
with.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index f6f5acd300a25fbc0defa737fe2eef9e011c07ef..4ecec35f2823c1f5bfb6b913b8035907f51298b8 100644 (file)
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3918,7 +3918,7 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
 
 /* caller must hold write lock */
 static int gnttab_get_status_frame_mfn(struct domain *d,
-                                       unsigned long idx, mfn_t *mfn)
+                                       unsigned int idx, mfn_t *mfn)
 {
     const struct grant_table *gt = d->grant_table;
 
@@ -3929,8 +3929,8 @@ static int gnttab_get_status_frame_mfn(struct domain *d,
 
     if ( idx >= nr_status_frames(gt) )
     {
-        unsigned long nr_status;
-        unsigned long nr_grant;
+        unsigned int nr_status;
+        unsigned int nr_grant;
 
         nr_status = idx + 1; /* sufficient frames to make idx valid */
 
@@ -3958,7 +3958,7 @@ static int gnttab_get_status_frame_mfn(struct domain *d,
 
 /* caller must hold write lock */
 static int gnttab_get_shared_frame_mfn(struct domain *d,
-                                       unsigned long idx, mfn_t *mfn)
+                                       unsigned int idx, mfn_t *mfn)
 {
     const struct grant_table *gt = d->grant_table;
 
@@ -3966,7 +3966,7 @@ static int gnttab_get_shared_frame_mfn(struct domain *d,
 
     if ( idx >= nr_grant_frames(gt) )
     {
-        unsigned long nr_grant;
+        unsigned int nr_grant;
 
         nr_grant = idx + 1; /* sufficient frames to make idx valid */
 
@@ -4021,7 +4021,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn, mfn_t *mfn)
     return rc;
 }
 
-int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
+int gnttab_get_shared_frame(struct domain *d, unsigned int idx,
                             mfn_t *mfn)
 {
     struct grant_table *gt = d->grant_table;
@@ -4034,7 +4034,7 @@ int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
     return rc;
 }
 
-int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+int gnttab_get_status_frame(struct domain *d, unsigned int idx,
                             mfn_t *mfn)
 {
     struct grant_table *gt = d->grant_table;

diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index a36137d41d1381dd0852aa759ceafc2789fa9483..90ed2e03024dd456f3ce39f736390b213d2265af 100644 (file)
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -771,7 +771,7 @@ static int ioreq_server_get_info(struct domain *d, ioservid_t id,
 }
 
 int ioreq_server_get_frame(struct domain *d, ioservid_t id,
-                           unsigned long idx, mfn_t *mfn)
+                           unsigned int idx, mfn_t *mfn)
 {
     struct ioreq_server *s;
     int rc;

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 33296e65cb04f1c48cd03769911d640e85762f50..4d681597a54f23b0962402b33a08ff8bb8732b8c 100644 (file)
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1055,7 +1055,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[])
 {
@@ -1094,7 +1094,7 @@ static int acquire_grant_table(struct domain *d, unsigned int id,
 
 static int acquire_ioreq_server(struct domain *d,
                                 unsigned int id,
-                                unsigned long frame,
+                                unsigned int frame,
                                 unsigned int nr_frames,
                                 xen_pfn_t mfn_list[])
 {
@@ -1164,6 +1164,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/xen/grant_table.h b/xen/include/xen/grant_table.h
index 8876f1f28efee800d5af10e905c1d9d35886ea44..6d14fe252636bde22ddef05c6773d850bee1c683 100644 (file)
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -55,9 +55,9 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
 
 int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
                      mfn_t *mfn);
-int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
+int gnttab_get_shared_frame(struct domain *d, unsigned int idx,
                             mfn_t *mfn);
-int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+int gnttab_get_status_frame(struct domain *d, unsigned int idx,
                             mfn_t *mfn);
 
 #else
@@ -92,13 +92,13 @@ static inline int gnttab_map_frame(struct domain *d, unsigned long idx,
     return -EINVAL;
 }
 
-static inline int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
+static inline int gnttab_get_shared_frame(struct domain *d, unsigned int idx,
                                           mfn_t *mfn)
 {
     return -EINVAL;
 }
 
-static inline int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+static inline int gnttab_get_status_frame(struct domain *d, unsigned int idx,
                                           mfn_t *mfn)
 {
     return -EINVAL;

diff --git a/xen/include/xen/ioreq.h b/xen/include/xen/ioreq.h
index 2d635e943214c07c2646e3fb4d48c73b203e5321..a54a637bef303c265530199ce527d710971394ea 100644 (file)
--- a/xen/include/xen/ioreq.h
+++ b/xen/include/xen/ioreq.h
@@ -90,7 +90,7 @@ bool vcpu_ioreq_handle_completion(struct vcpu *v);
 bool is_ioreq_server_page(struct domain *d, const struct page_info *page);
 
 int ioreq_server_get_frame(struct domain *d, ioservid_t id,
-                           unsigned long idx, mfn_t *mfn);
+                           unsigned int idx, mfn_t *mfn);
 int ioreq_server_map_mem_type(struct domain *d, ioservid_t id,
                               uint32_t type, uint32_t flags);