1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
|
From 3b896936f7505e929dd869d14afcb185d0ee75f8 Mon Sep 17 00:00:00 2001
From: Julien Grall <julien.grall@arm.com>
Date: Tue, 15 Oct 2019 17:10:41 +0100
Subject: [PATCH 2/3] xen/arm: p2m: Avoid off-by-one check on
p2m->max_mapped_gfn
The code base is using inconsistently the field p2m->max_mapped_gfn.
Some of the useres expect that p2m->max_guest_gfn contain the highest
mapped GFN while others expect highest + 1.
p2m->max_guest_gfn is set as highest + 1, because of that the sanity
check on the GFN in p2m_resolved_translation_fault() and
p2m_get_entry() can be bypassed when GFN == p2m->max_guest_gfn.
p2m_get_root_pointer(p2m->max_guest_gfn) may return NULL if it is
outside of address range supported and therefore the BUG_ON() could be
hit.
The current value hold in p2m->max_mapped_gfn is inconsistent with the
expectation of the common code (see domain_get_maximum_gpfn()) and also
the documentation of the field.
Rather than changing the check in p2m_translation_fault() and
p2m_get_entry(), p2m->max_mapped_gfn is now containing the highest
mapped GFN and the callers assuming "highest + 1" are now adjusted.
Take the opportunity to use 1UL rather than 1 as page_order could
theoritically big enough to overflow a 32-bit integer.
Lastly, the documentation of the field max_guest_gfn to reflect how it
is computed.
This is part of XSA-301.
Reported-by: Julien Grall <Julien.Grall@arm.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
xen/arch/arm/p2m.c | 6 +++---
xen/include/asm-arm/p2m.h | 5 +----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d0045a8b28..8d20d27961 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1041,7 +1041,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
p2m_write_pte(entry, pte, p2m->clean_pte);
p2m->max_mapped_gfn = gfn_max(p2m->max_mapped_gfn,
- gfn_add(sgfn, 1 << page_order));
+ gfn_add(sgfn, (1UL << page_order) - 1));
p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, sgfn);
}
@@ -1572,7 +1572,7 @@ int relinquish_p2m_mapping(struct domain *d)
p2m_write_lock(p2m);
start = p2m->lowest_mapped_gfn;
- end = p2m->max_mapped_gfn;
+ end = gfn_add(p2m->max_mapped_gfn, 1);
for ( ; gfn_x(start) < gfn_x(end);
start = gfn_next_boundary(start, order) )
@@ -1641,7 +1641,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t *pstart, gfn_t end)
p2m_read_lock(p2m);
start = gfn_max(start, p2m->lowest_mapped_gfn);
- end = gfn_min(end, p2m->max_mapped_gfn);
+ end = gfn_min(end, gfn_add(p2m->max_mapped_gfn, 1));
next_block_gfn = start;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 89f82df380..5fdb6e8183 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -36,10 +36,7 @@ struct p2m_domain {
/* Current Translation Table Base Register for the p2m */
uint64_t vttbr;
- /*
- * Highest guest frame that's ever been mapped in the p2m
- * Only takes into account ram and foreign mapping
- */
+ /* Highest guest frame that's ever been mapped in the p2m */
gfn_t max_mapped_gfn;
/*
--
2.23.0
|