| 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:43 +0100 |
| Subject: xen/gnttab: fix gnttab_end_foreign_access() without page specified |
| |
| From: Juergen Gross <jgross@suse.com> |
| |
| Commit 42baefac638f06314298087394b982ead9ec444b upstream. |
| |
| gnttab_end_foreign_access() is used to free a grant reference and |
| optionally to free the associated page. In case the grant is still in |
| use by the other side processing is being deferred. This leads to a |
| problem in case no page to be freed is specified by the caller: the |
| caller doesn't know that the page is still mapped by the other side |
| and thus should not be used for other purposes. |
| |
| The correct way to handle this situation is to take an additional |
| reference to the granted page in case handling is being deferred and |
| to drop that reference when the grant reference could be freed |
| finally. |
| |
| This requires that there are no users of gnttab_end_foreign_access() |
| left directly repurposing the granted page after the call, as this |
| might result in clobbered data or information leaks via the not yet |
| freed grant reference. |
| |
| This is part of CVE-2022-23041 / XSA-396. |
| |
| Reported-by: Simon Gaiser <simon@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/grant-table.c | 30 +++++++++++++++++++++++------- |
| include/xen/grant_table.h | 7 ++++++- |
| 2 files changed, 29 insertions(+), 8 deletions(-) |
| |
| --- a/drivers/xen/grant-table.c |
| +++ b/drivers/xen/grant-table.c |
| @@ -113,6 +113,10 @@ struct gnttab_ops { |
| * return the frame. |
| */ |
| unsigned long (*end_foreign_transfer_ref)(grant_ref_t ref); |
| + /* |
| + * Read the frame number related to a given grant reference. |
| + */ |
| + unsigned long (*read_frame)(grant_ref_t ref); |
| }; |
| |
| struct unmap_refs_callback_data { |
| @@ -277,6 +281,11 @@ int gnttab_end_foreign_access_ref(grant_ |
| } |
| EXPORT_SYMBOL_GPL(gnttab_end_foreign_access_ref); |
| |
| +static unsigned long gnttab_read_frame_v1(grant_ref_t ref) |
| +{ |
| + return gnttab_shared.v1[ref].frame; |
| +} |
| + |
| struct deferred_entry { |
| struct list_head list; |
| grant_ref_t ref; |
| @@ -306,12 +315,9 @@ static void gnttab_handle_deferred(unsig |
| spin_unlock_irqrestore(&gnttab_list_lock, flags); |
| if (_gnttab_end_foreign_access_ref(entry->ref, entry->ro)) { |
| put_free_entry(entry->ref); |
| - if (entry->page) { |
| - pr_debug("freeing g.e. %#x (pfn %#lx)\n", |
| - entry->ref, page_to_pfn(entry->page)); |
| - put_page(entry->page); |
| - } else |
| - pr_info("freeing g.e. %#x\n", entry->ref); |
| + pr_debug("freeing g.e. %#x (pfn %#lx)\n", |
| + entry->ref, page_to_pfn(entry->page)); |
| + put_page(entry->page); |
| kfree(entry); |
| entry = NULL; |
| } else { |
| @@ -336,9 +342,18 @@ static void gnttab_handle_deferred(unsig |
| static void gnttab_add_deferred(grant_ref_t ref, bool readonly, |
| struct page *page) |
| { |
| - struct deferred_entry *entry = kmalloc(sizeof(*entry), GFP_ATOMIC); |
| + struct deferred_entry *entry; |
| + gfp_t gfp = (in_atomic() || irqs_disabled()) ? GFP_ATOMIC : GFP_KERNEL; |
| const char *what = KERN_WARNING "leaking"; |
| |
| + entry = kmalloc(sizeof(*entry), gfp); |
| + if (!page) { |
| + unsigned long gfn = gnttab_interface->read_frame(ref); |
| + |
| + page = pfn_to_page(gfn_to_pfn(gfn)); |
| + get_page(page); |
| + } |
| + |
| if (entry) { |
| unsigned long flags; |
| |
| @@ -1010,6 +1025,7 @@ static const struct gnttab_ops gnttab_v1 |
| .update_entry = gnttab_update_entry_v1, |
| .end_foreign_access_ref = gnttab_end_foreign_access_ref_v1, |
| .end_foreign_transfer_ref = gnttab_end_foreign_transfer_ref_v1, |
| + .read_frame = gnttab_read_frame_v1, |
| }; |
| |
| static void gnttab_request_version(void) |
| --- a/include/xen/grant_table.h |
| +++ b/include/xen/grant_table.h |
| @@ -100,7 +100,12 @@ int gnttab_end_foreign_access_ref(grant_ |
| * Note that the granted page might still be accessed (read or write) by the |
| * other side after gnttab_end_foreign_access() returns, so even if page was |
| * specified as 0 it is not allowed to just reuse the page for other |
| - * purposes immediately. |
| + * purposes immediately. gnttab_end_foreign_access() will take an additional |
| + * reference to the granted page in this case, which is dropped only after |
| + * the grant is no longer in use. |
| + * This requires that multi page allocations for areas subject to |
| + * gnttab_end_foreign_access() are done via alloc_pages_exact() (and freeing |
| + * via free_pages_exact()) in order to avoid high order pages. |
| */ |
| void gnttab_end_foreign_access(grant_ref_t ref, int readonly, |
| unsigned long page); |