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 <demi@invisiblethingslab.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V3:
- fix __del_gref() (Jan Beulich)

diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
index 3fa40c723e8e95cb85d8c4ee5449ac716cb43e63..edb0acd0b8323cad2165d23fb4764358af8a45b8 100644 (file)
--- a/drivers/xen/gntalloc.c
+++ b/drivers/xen/gntalloc.c
@@ -169,20 +169,14 @@ undo:
                __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);
        return rc;
 }
 
 static void __del_gref(struct gntalloc_gref *gref)
 {
+       unsigned long addr;
+
        if (gref->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
                uint8_t *tmp = kmap(gref->page);
                tmp[gref->notify.pgoff] = 0;
@@ -196,21 +190,16 @@ static void __del_gref(struct gntalloc_gref *gref)
        gref->notify.flags = 0;
 
        if (gref->gref_id) {
-               if (gnttab_query_foreign_access(gref->gref_id))
-                       return;
-
-               if (!gnttab_end_foreign_access_ref(gref->gref_id, 0))
-                       return;
-
-               gnttab_free_grant_reference(gref->gref_id);
+               if (gref->page) {
+                       addr = (unsigned long)page_to_virt(gref->page);
+                       gnttab_end_foreign_access(gref->gref_id, 0, addr);
+               } else
+                       gnttab_free_grant_reference(gref->gref_id);
        }
 
        gref_size--;
        list_del(&gref->next_gref);
 
-       if (gref->page)
-               __free_page(gref->page);
-
        kfree(gref);
 }