From: Jan Beulich <jbeulich@suse.com>
Subject: 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>
---
v3: Don't init the ARM GFN arrays to zero anymore, use INVALID_GFN.
v2: New.
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3775,6 +3775,7 @@ int gnttab_map_frame(struct domain *d, u
{
int rc = 0;
struct grant_table *gt = d->grant_table;
+ bool status = false;
grant_write_lock(gt);
@@ -3785,6 +3786,7 @@ int gnttab_map_frame(struct domain *d, u
(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
@@ -3802,7 +3804,7 @@ int gnttab_map_frame(struct domain *d, u
}
if ( !rc )
- gnttab_set_frame_gfn(gt, idx, gfn);
+ gnttab_set_frame_gfn(gt, status, idx, gfn);
grant_write_unlock(gt);
--- 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
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_m
#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_m
} 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))
--- 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_m
#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 { \
|
