Commit d3b6372c authored by Juergen Gross's avatar Juergen Gross

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: default avatarDemi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: default avatarJuergen Gross <jgross@suse.com>
Reviewed-by: default avatarJan Beulich <jbeulich@suse.com>
---
V3:
- fix __del_gref() (Jan Beulich)
parent 33172ab5
...@@ -169,20 +169,14 @@ static int add_grefs(struct ioctl_gntalloc_alloc_gref *op, ...@@ -169,20 +169,14 @@ static int add_grefs(struct ioctl_gntalloc_alloc_gref *op,
__del_gref(gref); __del_gref(gref);
} }
/* It's possible for the target domain to map the just-allocated grant
* references by blindly guessing their IDs; if this is done, then
* __del_gref will leave them in the queue_gref list. They need to be
* added to the global list so that we can free them when they are no
* longer referenced.
*/
if (unlikely(!list_empty(&queue_gref)))
list_splice_tail(&queue_gref, &gref_list);
mutex_unlock(&gref_mutex); mutex_unlock(&gref_mutex);
return rc; return rc;
} }
static void __del_gref(struct gntalloc_gref *gref) static void __del_gref(struct gntalloc_gref *gref)
{ {
unsigned long addr;
if (gref->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) { if (gref->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
uint8_t *tmp = kmap(gref->page); uint8_t *tmp = kmap(gref->page);
tmp[gref->notify.pgoff] = 0; tmp[gref->notify.pgoff] = 0;
...@@ -196,21 +190,16 @@ static void __del_gref(struct gntalloc_gref *gref) ...@@ -196,21 +190,16 @@ static void __del_gref(struct gntalloc_gref *gref)
gref->notify.flags = 0; gref->notify.flags = 0;
if (gref->gref_id) { if (gref->gref_id) {
if (gnttab_query_foreign_access(gref->gref_id)) if (gref->page) {
return; addr = (unsigned long)page_to_virt(gref->page);
gnttab_end_foreign_access(gref->gref_id, 0, addr);
if (!gnttab_end_foreign_access_ref(gref->gref_id, 0)) } else
return; gnttab_free_grant_reference(gref->gref_id);
gnttab_free_grant_reference(gref->gref_id);
} }
gref_size--; gref_size--;
list_del(&gref->next_gref); list_del(&gref->next_gref);
if (gref->page)
__free_page(gref->page);
kfree(gref); kfree(gref);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment