gnttab/ARM: don't corrupt shared GFN array

... by writing status GFNs to it. Introduce a second array instead.
Also implement gnttab_status_gmfn() properly now that the information is
suitably being tracked.

While touching it anyway, remove a misguided (but luckily benign) upper
bound check from gnttab_shared_gmfn(): We should never access beyond the
bounds of that array.

This is part of XSA-255.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 8346994de623015383a44c52017ab5903a8daf47..43ec69d5190cd17ca2ee0d810c350603c0cfaa4b 100644 (file)
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3776,6 +3776,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
 {
     int rc = 0;
     struct grant_table *gt = d->grant_table;
+    bool status = false;
 
     grant_write_lock(gt);
 
@@ -3786,6 +3787,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
          (idx & XENMAPIDX_grant_table_status) )
     {
         idx &= ~XENMAPIDX_grant_table_status;
+        status = true;
         if ( idx < nr_status_frames(gt) )
             *mfn = _mfn(virt_to_mfn(gt->status[idx]));
         else
@@ -3803,7 +3805,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
     }
 
     if ( !rc )
-        gnttab_set_frame_gfn(gt, idx, gfn);
+        gnttab_set_frame_gfn(gt, status, idx, gfn);
 
     grant_write_unlock(gt);
 

diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index 0dfdc5577fe5c9e7f9a4dda4848813a83ba6e7d6..f5361c7dcf9ed611904c96b15bd764d3f64b0917 100644 (file)
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -9,7 +9,8 @@
 #define INITIAL_NR_GRANT_FRAMES 1U
 
 struct grant_table_arch {
-    gfn_t *gfn;
+    gfn_t *shared_gfn;
+    gfn_t *status_gfn;
 };
 
 void gnttab_clear_flag(unsigned long nr, uint16_t *addr);
@@ -21,7 +22,6 @@ int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
         unsigned long new_gpaddr, unsigned int flags);
 void gnttab_mark_dirty(struct domain *d, unsigned long l);
 #define gnttab_create_status_page(d, t, i) do {} while (0)
-#define gnttab_status_gmfn(d, t, i) (0)
 #define gnttab_release_host_mappings(domain) 1
 static inline int replace_grant_supported(void)
 {
@@ -42,19 +42,35 @@ static inline unsigned int gnttab_dom0_max(void)
 
 #define gnttab_init_arch(gt)                                             \
 ({                                                                       \
-    (gt)->arch.gfn = xzalloc_array(gfn_t, (gt)->max_grant_frames);       \
-    ( (gt)->arch.gfn ? 0 : -ENOMEM );                                    \
+    unsigned int ngf_ = (gt)->max_grant_frames;                          \
+    unsigned int nsf_ = grant_to_status_frames(ngf_);                    \
+                                                                         \
+    (gt)->arch.shared_gfn = xmalloc_array(gfn_t, ngf_);                  \
+    (gt)->arch.status_gfn = xmalloc_array(gfn_t, nsf_);                  \
+    if ( (gt)->arch.shared_gfn && (gt)->arch.status_gfn )                \
+    {                                                                    \
+        while ( ngf_-- )                                                 \
+            (gt)->arch.shared_gfn[ngf_] = INVALID_GFN;                   \
+        while ( nsf_-- )                                                 \
+            (gt)->arch.status_gfn[nsf_] = INVALID_GFN;                   \
+    }                                                                    \
+    else                                                                 \
+        gnttab_destroy_arch(gt);                                         \
+    (gt)->arch.shared_gfn ? 0 : -ENOMEM;                                 \
 })
 
 #define gnttab_destroy_arch(gt)                                          \
     do {                                                                 \
-        xfree((gt)->arch.gfn);                                           \
-        (gt)->arch.gfn = NULL;                                           \
+        xfree((gt)->arch.shared_gfn);                                    \
+        (gt)->arch.shared_gfn = NULL;                                    \
+        xfree((gt)->arch.status_gfn);                                    \
+        (gt)->arch.status_gfn = NULL;                                    \
     } while ( 0 )
 
-#define gnttab_set_frame_gfn(gt, idx, gfn)                               \
+#define gnttab_set_frame_gfn(gt, st, idx, gfn)                           \
     do {                                                                 \
-        (gt)->arch.gfn[idx] = gfn;                                       \
+        ((st) ? (gt)->arch.status_gfn : (gt)->arch.shared_gfn)[idx] =    \
+            (gfn);                                                       \
     } while ( 0 )
 
 #define gnttab_create_shared_page(d, t, i)                               \
@@ -65,8 +81,10 @@ static inline unsigned int gnttab_dom0_max(void)
     } while ( 0 )
 
 #define gnttab_shared_gmfn(d, t, i)                                      \
-    ( ((i >= nr_grant_frames(t)) &&                                      \
-       (i < (t)->max_grant_frames))? 0 : gfn_x((t)->arch.gfn[i]))
+    gfn_x(((i) >= nr_grant_frames(t)) ? INVALID_GFN : (t)->arch.shared_gfn[i])
+
+#define gnttab_status_gmfn(d, t, i)                                      \
+    gfn_x(((i) >= nr_status_frames(t)) ? INVALID_GFN : (t)->arch.status_gfn[i])
 
 #define gnttab_need_iommu_mapping(d)                    \
     (is_domain_direct_mapped(d) && need_iommu(d))

diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index d9157e441712a1ade91eb5ad68067aa0c56f0430..3177d82b69bb4590e62dc5faf0c4021006035a12 100644 (file)
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -46,7 +46,7 @@ static inline unsigned int gnttab_dom0_max(void)
 
 #define gnttab_init_arch(gt) 0
 #define gnttab_destroy_arch(gt) do {} while ( 0 )
-#define gnttab_set_frame_gfn(gt, idx, gfn) do {} while ( 0 )
+#define gnttab_set_frame_gfn(gt, st, idx, gfn) do {} while ( 0 )
 
 #define gnttab_create_shared_page(d, t, i)                               \
     do {                                                                 \