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
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
|
From: Jan Beulich <jbeulich@suse.com>
Subject: gnttab: also validate PTE permissions upon destroy/replace
In order for PTE handling to match up with the reference counting done
by common code, presence and writability of grant mapping PTEs must
also be taken into account; validating just the frame number is not
enough. This is in particular relevant if a guest fiddles with grant
PTEs via non-grant hypercalls.
Note that the flags being passed to replace_grant_host_mapping()
already happen to be those of the existing mapping, so no new function
parameter is needed.
This is XSA-234.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4058,7 +4058,8 @@ static int create_grant_pte_mapping(
}
static int destroy_grant_pte_mapping(
- uint64_t addr, unsigned long frame, struct domain *d)
+ uint64_t addr, unsigned long frame, unsigned int grant_pte_flags,
+ struct domain *d)
{
int rc = GNTST_okay;
void *va;
@@ -4104,17 +4105,29 @@ static int destroy_grant_pte_mapping(
ol1e = *(l1_pgentry_t *)va;
- /* Check that the virtual address supplied is actually mapped to frame. */
- if ( unlikely(l1e_get_pfn(ol1e) != frame) )
+ /*
+ * Check that the PTE supplied actually maps frame (with appropriate
+ * permissions).
+ */
+ if ( unlikely(l1e_get_pfn(ol1e) != frame) ||
+ unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
+ (_PAGE_PRESENT | _PAGE_RW)) )
{
page_unlock(page);
- gdprintk(XENLOG_WARNING,
- "PTE entry %"PRIpte" for address %"PRIx64" doesn't match frame %lx\n",
- l1e_get_intpte(ol1e), addr, frame);
+ gdprintk(XENLOG_ERR,
+ "PTE %"PRIpte" at %"PRIx64" doesn't match grant (%"PRIpte")\n",
+ l1e_get_intpte(ol1e), addr,
+ l1e_get_intpte(l1e_from_pfn(frame, grant_pte_flags)));
rc = GNTST_general_error;
goto failed;
}
+ if ( unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
+ ~(_PAGE_AVAIL | PAGE_CACHE_ATTRS)) )
+ gdprintk(XENLOG_WARNING,
+ "PTE flags %x at %"PRIx64" don't match grant (%x)\n",
+ l1e_get_flags(ol1e), addr, grant_pte_flags);
+
/* Delete pagetable entry. */
if ( unlikely(!UPDATE_ENTRY
(l1,
@@ -4123,7 +4136,8 @@ static int destroy_grant_pte_mapping(
0)) )
{
page_unlock(page);
- gdprintk(XENLOG_WARNING, "Cannot delete PTE entry at %p\n", va);
+ gdprintk(XENLOG_WARNING, "Cannot delete PTE entry at %"PRIx64"\n",
+ addr);
rc = GNTST_general_error;
goto failed;
}
@@ -4191,7 +4205,8 @@ static int create_grant_va_mapping(
}
static int replace_grant_va_mapping(
- unsigned long addr, unsigned long frame, l1_pgentry_t nl1e, struct vcpu *v)
+ unsigned long addr, unsigned long frame, unsigned int grant_pte_flags,
+ l1_pgentry_t nl1e, struct vcpu *v)
{
l1_pgentry_t *pl1e, ol1e;
unsigned long gl1mfn;
@@ -4227,20 +4242,33 @@ static int replace_grant_va_mapping(
ol1e = *pl1e;
- /* Check that the virtual address supplied is actually mapped to frame. */
- if ( unlikely(l1e_get_pfn(ol1e) != frame) )
- {
- gdprintk(XENLOG_WARNING,
- "PTE entry %lx for address %lx doesn't match frame %lx\n",
- l1e_get_pfn(ol1e), addr, frame);
+ /*
+ * Check that the virtual address supplied is actually mapped to frame
+ * (with appropriate permissions).
+ */
+ if ( unlikely(l1e_get_pfn(ol1e) != frame) ||
+ unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
+ (_PAGE_PRESENT | _PAGE_RW)) )
+ {
+ gdprintk(XENLOG_ERR,
+ "PTE %"PRIpte" for %lx doesn't match grant (%"PRIpte")\n",
+ l1e_get_intpte(ol1e), addr,
+ l1e_get_intpte(l1e_from_pfn(frame, grant_pte_flags)));
rc = GNTST_general_error;
goto unlock_and_out;
}
+ if ( unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
+ ~(_PAGE_AVAIL | PAGE_CACHE_ATTRS)) )
+ gdprintk(XENLOG_WARNING,
+ "PTE flags %x for %"PRIx64" don't match grant (%x)\n",
+ l1e_get_flags(ol1e), addr, grant_pte_flags);
+
/* Delete pagetable entry. */
if ( unlikely(!UPDATE_ENTRY(l1, pl1e, ol1e, nl1e, gl1mfn, v, 0)) )
{
- gdprintk(XENLOG_WARNING, "Cannot delete PTE entry at %p\n", pl1e);
+ gdprintk(XENLOG_WARNING, "Cannot delete PTE entry for %"PRIx64"\n",
+ addr);
rc = GNTST_general_error;
goto unlock_and_out;
}
@@ -4254,9 +4282,11 @@ static int replace_grant_va_mapping(
}
static int destroy_grant_va_mapping(
- unsigned long addr, unsigned long frame, struct vcpu *v)
+ unsigned long addr, unsigned long frame, unsigned int grant_pte_flags,
+ struct vcpu *v)
{
- return replace_grant_va_mapping(addr, frame, l1e_empty(), v);
+ return replace_grant_va_mapping(addr, frame, grant_pte_flags,
+ l1e_empty(), v);
}
static int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
@@ -4351,20 +4381,39 @@ int replace_grant_host_mapping(
unsigned long gl1mfn;
struct page_info *l1pg;
int rc;
+ unsigned int grant_pte_flags;
if ( paging_mode_external(current->domain) )
return replace_grant_p2m_mapping(addr, frame, new_addr, flags);
+ grant_pte_flags =
+ _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_DIRTY | _PAGE_GNTTAB | _PAGE_NX;
+
+ if ( flags & GNTMAP_application_map )
+ grant_pte_flags |= _PAGE_USER;
+ if ( !(flags & GNTMAP_readonly) )
+ grant_pte_flags |= _PAGE_RW;
+ /*
+ * On top of the explicit settings done by create_grant_host_mapping()
+ * also open-code relevant parts of adjust_guest_l1e(). Don't mirror
+ * available and cachability flags, though.
+ */
+ if ( !is_pv_32bit_domain(curr->domain) )
+ grant_pte_flags |= (grant_pte_flags & _PAGE_USER)
+ ? _PAGE_GLOBAL
+ : _PAGE_GUEST_KERNEL | _PAGE_USER;
+
if ( flags & GNTMAP_contains_pte )
{
if ( !new_addr )
- return destroy_grant_pte_mapping(addr, frame, curr->domain);
+ return destroy_grant_pte_mapping(addr, frame, grant_pte_flags,
+ curr->domain);
return GNTST_general_error;
}
if ( !new_addr )
- return destroy_grant_va_mapping(addr, frame, curr);
+ return destroy_grant_va_mapping(addr, frame, grant_pte_flags, curr);
pl1e = guest_map_l1e(new_addr, &gl1mfn);
if ( !pl1e )
@@ -4412,7 +4461,7 @@ int replace_grant_host_mapping(
put_page(l1pg);
guest_unmap_l1e(pl1e);
- rc = replace_grant_va_mapping(addr, frame, ol1e, curr);
+ rc = replace_grant_va_mapping(addr, frame, grant_pte_flags, ol1e, curr);
if ( rc && !paging_mode_refcounts(curr->domain) )
put_page_from_l1e(ol1e, curr->domain);
|
