| From foo@baz Thu Mar 10 02:41:19 PM CET 2022 |
| From: Juergen Gross <jgross@suse.com> |
| Date: Fri, 25 Feb 2022 16:05:42 +0100 |
| Subject: xen/gntalloc: don't use gnttab_query_foreign_access() |
| |
| From: Juergen Gross <jgross@suse.com> |
| |
| Commit d3b6372c5881cb54925212abb62c521df8ba4809 upstream. |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/xen/gntalloc.c | 25 +++++++------------------ |
| 1 file changed, 7 insertions(+), 18 deletions(-) |
| |
| --- a/drivers/xen/gntalloc.c |
| +++ b/drivers/xen/gntalloc.c |
| @@ -166,20 +166,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; |
| @@ -193,21 +187,16 @@ static void __del_gref(struct gntalloc_g |
| 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); |
| } |
| |