|
Juergen Gross |
20b798 |
Patch-mainline: v5.17
|
|
Juergen Gross |
20b798 |
Git-commit: 42baefac638f06314298087394b982ead9ec444b
|
|
Juergen Gross |
20b798 |
References: bsc#1196488, XSA-396, CVE-2022-23041
|
|
Juergen Gross |
20b798 |
From: Juergen Gross <jgross@suse.com>
|
|
Juergen Gross |
20b798 |
Date: Mon, 7 Mar 2022 09:48:55 +0100
|
|
Juergen Gross |
20b798 |
Subject: [PATCH] xen/gnttab: fix gnttab_end_foreign_access() without page
|
|
Juergen Gross |
20b798 |
specified
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
gnttab_end_foreign_access() is used to free a grant reference and
|
|
Juergen Gross |
20b798 |
optionally to free the associated page. In case the grant is still in
|
|
Juergen Gross |
20b798 |
use by the other side processing is being deferred. This leads to a
|
|
Juergen Gross |
20b798 |
problem in case no page to be freed is specified by the caller: the
|
|
Juergen Gross |
20b798 |
caller doesn't know that the page is still mapped by the other side
|
|
Juergen Gross |
20b798 |
and thus should not be used for other purposes.
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
The correct way to handle this situation is to take an additional
|
|
Juergen Gross |
20b798 |
reference to the granted page in case handling is being deferred and
|
|
Juergen Gross |
20b798 |
to drop that reference when the grant reference could be freed
|
|
Juergen Gross |
20b798 |
finally.
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
This requires that there are no users of gnttab_end_foreign_access()
|
|
Juergen Gross |
20b798 |
left directly repurposing the granted page after the call, as this
|
|
Juergen Gross |
20b798 |
might result in clobbered data or information leaks via the not yet
|
|
Juergen Gross |
20b798 |
freed grant reference.
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
This is part of CVE-2022-23041 / XSA-396.
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
Reported-by: Simon Gaiser <simon@invisiblethingslab.com>
|
|
Juergen Gross |
20b798 |
Signed-off-by: Juergen Gross <jgross@suse.com>
|
|
Juergen Gross |
20b798 |
Reviewed-by: Jan Beulich <jbeulich@suse.com>
|
|
Juergen Gross |
20b798 |
---
|
|
Juergen Gross |
20b798 |
V4:
|
|
Juergen Gross |
20b798 |
- expand comment in header
|
|
Juergen Gross |
20b798 |
V5:
|
|
Juergen Gross |
20b798 |
- get page ref in case of kmalloc() failure, too
|
|
Juergen Gross |
20b798 |
---
|
|
Juergen Gross |
20b798 |
drivers/xen/grant-table.c | 36 +++++++++++++++++++++++++++++-------
|
|
Juergen Gross |
20b798 |
include/xen/grant_table.h | 7 ++++++-
|
|
Juergen Gross |
20b798 |
2 files changed, 35 insertions(+), 8 deletions(-)
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
|
|
Juergen Gross |
20b798 |
index e6548910e79f..5c83d41766c8 100644
|
|
Juergen Gross |
20b798 |
--- a/drivers/xen/grant-table.c
|
|
Juergen Gross |
20b798 |
+++ b/drivers/xen/grant-table.c
|
|
Juergen Gross |
20b798 |
@@ -133,6 +133,10 @@ struct gnttab_ops {
|
|
Juergen Gross |
20b798 |
* by bit operations.
|
|
Juergen Gross |
20b798 |
*/
|
|
Juergen Gross |
20b798 |
int (*query_foreign_access)(grant_ref_t ref);
|
|
Juergen Gross |
20b798 |
+ /*
|
|
Juergen Gross |
20b798 |
+ * Read the frame number related to a given grant reference.
|
|
Juergen Gross |
20b798 |
+ */
|
|
Juergen Gross |
20b798 |
+ unsigned long (*read_frame)(grant_ref_t ref);
|
|
Juergen Gross |
20b798 |
};
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
struct unmap_refs_callback_data {
|
|
Juergen Gross |
20b798 |
@@ -330,6 +334,16 @@ int gnttab_end_foreign_access_ref(grant_ref_t ref, int readonly)
|
|
Juergen Gross |
20b798 |
}
|
|
Juergen Gross |
20b798 |
EXPORT_SYMBOL_GPL(gnttab_end_foreign_access_ref);
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
+static unsigned long gnttab_read_frame_v1(grant_ref_t ref)
|
|
Juergen Gross |
20b798 |
+{
|
|
Juergen Gross |
20b798 |
+ return gnttab_shared.v1[ref].frame;
|
|
Juergen Gross |
20b798 |
+}
|
|
Juergen Gross |
20b798 |
+
|
|
Juergen Gross |
20b798 |
+static unsigned long gnttab_read_frame_v2(grant_ref_t ref)
|
|
Juergen Gross |
20b798 |
+{
|
|
Juergen Gross |
20b798 |
+ return gnttab_shared.v2[ref].full_page.frame;
|
|
Juergen Gross |
20b798 |
+}
|
|
Juergen Gross |
20b798 |
+
|
|
Juergen Gross |
20b798 |
struct deferred_entry {
|
|
Juergen Gross |
20b798 |
struct list_head list;
|
|
Juergen Gross |
20b798 |
grant_ref_t ref;
|
|
Juergen Gross |
20b798 |
@@ -359,12 +373,9 @@ static void gnttab_handle_deferred(struct timer_list *unused)
|
|
Juergen Gross |
20b798 |
spin_unlock_irqrestore(&gnttab_list_lock, flags);
|
|
Juergen Gross |
20b798 |
if (_gnttab_end_foreign_access_ref(entry->ref, entry->ro)) {
|
|
Juergen Gross |
20b798 |
put_free_entry(entry->ref);
|
|
Juergen Gross |
20b798 |
- if (entry->page) {
|
|
Juergen Gross |
20b798 |
- pr_debug("freeing g.e. %#x (pfn %#lx)\n",
|
|
Juergen Gross |
20b798 |
- entry->ref, page_to_pfn(entry->page));
|
|
Juergen Gross |
20b798 |
- put_page(entry->page);
|
|
Juergen Gross |
20b798 |
- } else
|
|
Juergen Gross |
20b798 |
- pr_info("freeing g.e. %#x\n", entry->ref);
|
|
Juergen Gross |
20b798 |
+ pr_debug("freeing g.e. %#x (pfn %#lx)\n",
|
|
Juergen Gross |
20b798 |
+ entry->ref, page_to_pfn(entry->page));
|
|
Juergen Gross |
20b798 |
+ put_page(entry->page);
|
|
Juergen Gross |
20b798 |
kfree(entry);
|
|
Juergen Gross |
20b798 |
entry = NULL;
|
|
Juergen Gross |
20b798 |
} else {
|
|
Juergen Gross |
20b798 |
@@ -389,9 +400,18 @@ static void gnttab_handle_deferred(struct timer_list *unused)
|
|
Juergen Gross |
20b798 |
static void gnttab_add_deferred(grant_ref_t ref, bool readonly,
|
|
Juergen Gross |
20b798 |
struct page *page)
|
|
Juergen Gross |
20b798 |
{
|
|
Juergen Gross |
20b798 |
- struct deferred_entry *entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
|
|
Juergen Gross |
20b798 |
+ struct deferred_entry *entry;
|
|
Juergen Gross |
20b798 |
+ gfp_t gfp = (in_atomic() || irqs_disabled()) ? GFP_ATOMIC : GFP_KERNEL;
|
|
Juergen Gross |
20b798 |
const char *what = KERN_WARNING "leaking";
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
+ entry = kmalloc(sizeof(*entry), gfp);
|
|
Juergen Gross |
20b798 |
+ if (!page) {
|
|
Juergen Gross |
20b798 |
+ unsigned long gfn = gnttab_interface->read_frame(ref);
|
|
Juergen Gross |
20b798 |
+
|
|
Juergen Gross |
20b798 |
+ page = pfn_to_page(gfn_to_pfn(gfn));
|
|
Juergen Gross |
20b798 |
+ get_page(page);
|
|
Juergen Gross |
20b798 |
+ }
|
|
Juergen Gross |
20b798 |
+
|
|
Juergen Gross |
20b798 |
if (entry) {
|
|
Juergen Gross |
20b798 |
unsigned long flags;
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
@@ -1404,6 +1424,7 @@ static const struct gnttab_ops gnttab_v1_ops = {
|
|
Juergen Gross |
20b798 |
.end_foreign_access_ref = gnttab_end_foreign_access_ref_v1,
|
|
Juergen Gross |
20b798 |
.end_foreign_transfer_ref = gnttab_end_foreign_transfer_ref_v1,
|
|
Juergen Gross |
20b798 |
.query_foreign_access = gnttab_query_foreign_access_v1,
|
|
Juergen Gross |
20b798 |
+ .read_frame = gnttab_read_frame_v1,
|
|
Juergen Gross |
20b798 |
};
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
static const struct gnttab_ops gnttab_v2_ops = {
|
|
Juergen Gross |
20b798 |
@@ -1415,6 +1436,7 @@ static const struct gnttab_ops gnttab_v2_ops = {
|
|
Juergen Gross |
20b798 |
.end_foreign_access_ref = gnttab_end_foreign_access_ref_v2,
|
|
Juergen Gross |
20b798 |
.end_foreign_transfer_ref = gnttab_end_foreign_transfer_ref_v2,
|
|
Juergen Gross |
20b798 |
.query_foreign_access = gnttab_query_foreign_access_v2,
|
|
Juergen Gross |
20b798 |
+ .read_frame = gnttab_read_frame_v2,
|
|
Juergen Gross |
20b798 |
};
|
|
Juergen Gross |
20b798 |
|
|
Juergen Gross |
20b798 |
static bool gnttab_need_v2(void)
|
|
Juergen Gross |
20b798 |
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
|
|
Juergen Gross |
20b798 |
index ab9e692a0ef4..c9fea9389ebe 100644
|
|
Juergen Gross |
20b798 |
--- a/include/xen/grant_table.h
|
|
Juergen Gross |
20b798 |
+++ b/include/xen/grant_table.h
|
|
Juergen Gross |
20b798 |
@@ -107,7 +107,12 @@ int gnttab_end_foreign_access_ref(grant_ref_t ref, int readonly);
|
|
Juergen Gross |
20b798 |
* Note that the granted page might still be accessed (read or write) by the
|
|
Juergen Gross |
20b798 |
* other side after gnttab_end_foreign_access() returns, so even if page was
|
|
Juergen Gross |
20b798 |
* specified as 0 it is not allowed to just reuse the page for other
|
|
Juergen Gross |
20b798 |
- * purposes immediately.
|
|
Juergen Gross |
20b798 |
+ * purposes immediately. gnttab_end_foreign_access() will take an additional
|
|
Juergen Gross |
20b798 |
+ * reference to the granted page in this case, which is dropped only after
|
|
Juergen Gross |
20b798 |
+ * the grant is no longer in use.
|
|
Juergen Gross |
20b798 |
+ * This requires that multi page allocations for areas subject to
|
|
Juergen Gross |
20b798 |
+ * gnttab_end_foreign_access() are done via alloc_pages_exact() (and freeing
|
|
Juergen Gross |
20b798 |
+ * via free_pages_exact()) in order to avoid high order pages.
|
|
Juergen Gross |
20b798 |
*/
|
|
Juergen Gross |
20b798 |
void gnttab_end_foreign_access(grant_ref_t ref, int readonly,
|
|
Juergen Gross |
20b798 |
unsigned long page);
|
|
Juergen Gross |
20b798 |
--
|
|
Juergen Gross |
20b798 |
2.34.1
|
|
Juergen Gross |
20b798 |
|