Skip to content

Commit d3b6372

Browse files
committed
xen/gntalloc: don't use gnttab_query_foreign_access()
Using gnttab_query_foreign_access() is unsafe, as it is racy by design. The use case in the gntalloc driver is not needed at all. While at it replace the call of gnttab_end_foreign_access_ref() with a call of gnttab_end_foreign_access(), which is what is really wanted there. In case the grant wasn't used due to an allocation failure, just free the grant via gnttab_free_grant_reference(). This is CVE-2022-23039 / part of XSA-396. Reported-by: Demi Marie Obenour <[email protected]> Signed-off-by: Juergen Gross <[email protected]> Reviewed-by: Jan Beulich <[email protected]> --- V3: - fix __del_gref() (Jan Beulich)
1 parent 33172ab commit d3b6372

File tree

1 file changed

+7
-18
lines changed

1 file changed

+7
-18
lines changed

drivers/xen/gntalloc.c

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -169,20 +169,14 @@ static int add_grefs(struct ioctl_gntalloc_alloc_gref *op,
169169
__del_gref(gref);
170170
}
171171

172-
/* It's possible for the target domain to map the just-allocated grant
173-
* references by blindly guessing their IDs; if this is done, then
174-
* __del_gref will leave them in the queue_gref list. They need to be
175-
* added to the global list so that we can free them when they are no
176-
* longer referenced.
177-
*/
178-
if (unlikely(!list_empty(&queue_gref)))
179-
list_splice_tail(&queue_gref, &gref_list);
180172
mutex_unlock(&gref_mutex);
181173
return rc;
182174
}
183175

184176
static void __del_gref(struct gntalloc_gref *gref)
185177
{
178+
unsigned long addr;
179+
186180
if (gref->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
187181
uint8_t *tmp = kmap(gref->page);
188182
tmp[gref->notify.pgoff] = 0;
@@ -196,21 +190,16 @@ static void __del_gref(struct gntalloc_gref *gref)
196190
gref->notify.flags = 0;
197191

198192
if (gref->gref_id) {
199-
if (gnttab_query_foreign_access(gref->gref_id))
200-
return;
201-
202-
if (!gnttab_end_foreign_access_ref(gref->gref_id, 0))
203-
return;
204-
205-
gnttab_free_grant_reference(gref->gref_id);
193+
if (gref->page) {
194+
addr = (unsigned long)page_to_virt(gref->page);
195+
gnttab_end_foreign_access(gref->gref_id, 0, addr);
196+
} else
197+
gnttab_free_grant_reference(gref->gref_id);
206198
}
207199

208200
gref_size--;
209201
list_del(&gref->next_gref);
210202

211-
if (gref->page)
212-
__free_page(gref->page);
213-
214203
kfree(gref);
215204
}
216205

0 commit comments

Comments
 (0)