111 lines
3.9 KiB
Diff
111 lines
3.9 KiB
Diff
From fd97f5f5ba9375163c8d8771fe551bb4a6423b36 Mon Sep 17 00:00:00 2001
|
|
From: George Dunlap <george.dunlap@citrix.com>
|
|
Date: Thu, 15 Jun 2017 16:24:02 +0100
|
|
Subject: [PATCH 1/4] gnttab: Fix handling of dev_bus_addr during unmap
|
|
|
|
If a grant has been mapped with the GNTTAB_device_map flag, calling
|
|
grant_unmap_ref() with dev_bus_addr set to zero should cause the
|
|
GNTTAB_device_map part of the mapping to be left alone.
|
|
|
|
Unfortunately, at the moment, op->dev_bus_addr is implicitly checked
|
|
before clearing the map and adjusting the pin count, but only the bits
|
|
above 12; and it is not checked at all before dropping page
|
|
references. This means a guest can repeatedly make such a call to
|
|
cause the reference count to drop to zero, causing the page to be
|
|
freed and re-used, even though it's still mapped in its pagetables.
|
|
|
|
To fix this, always check op->dev_bus_addr explicitly for being
|
|
non-zero, as well as op->flag & GNTMAP_device_map, before doing
|
|
operations on the device_map.
|
|
|
|
While we're here, make the logic a bit cleaner:
|
|
|
|
* Always initialize op->frame to zero and set it from act->frame, to reduce the
|
|
chance of untrusted input being used
|
|
|
|
* Explicitly check the full dev_bus_addr against act->frame <<
|
|
PAGE_SHIFT, rather than ignoring the lower 12 bits
|
|
|
|
This is part of XSA-224.
|
|
|
|
Reported-by: Jan Beulich <jbeulich@suse.com>
|
|
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
|
|
Signed-off-by: Jan Beulich <jbeulich@suse.com>
|
|
---
|
|
xen/common/grant_table.c | 23 +++++++++++------------
|
|
1 file changed, 11 insertions(+), 12 deletions(-)
|
|
|
|
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
|
|
index c4d73af..69cbdb6 100644
|
|
--- a/xen/common/grant_table.c
|
|
+++ b/xen/common/grant_table.c
|
|
@@ -1089,8 +1089,6 @@ __gnttab_unmap_common(
|
|
ld = current->domain;
|
|
lgt = ld->grant_table;
|
|
|
|
- op->frame = (unsigned long)(op->dev_bus_addr >> PAGE_SHIFT);
|
|
-
|
|
if ( unlikely(op->handle >= lgt->maptrack_limit) )
|
|
{
|
|
gdprintk(XENLOG_INFO, "Bad handle (%d).\n", op->handle);
|
|
@@ -1174,16 +1172,14 @@ __gnttab_unmap_common(
|
|
goto act_release_out;
|
|
}
|
|
|
|
- if ( op->frame == 0 )
|
|
- {
|
|
- op->frame = act->frame;
|
|
- }
|
|
- else
|
|
+ op->frame = act->frame;
|
|
+
|
|
+ if ( op->dev_bus_addr )
|
|
{
|
|
- if ( unlikely(op->frame != act->frame) )
|
|
+ if ( unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
|
|
PIN_FAIL(act_release_out, GNTST_general_error,
|
|
- "Bad frame number doesn't match gntref. (%lx != %lx)\n",
|
|
- op->frame, act->frame);
|
|
+ "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n",
|
|
+ op->dev_bus_addr, pfn_to_paddr(act->frame));
|
|
|
|
map->flags &= ~GNTMAP_device_map;
|
|
}
|
|
@@ -1276,7 +1272,8 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
|
|
else
|
|
status = &status_entry(rgt, op->ref);
|
|
|
|
- if ( unlikely(op->frame != act->frame) )
|
|
+ if ( op->dev_bus_addr &&
|
|
+ unlikely(op->dev_bus_addr != pfn_to_paddr(act->frame)) )
|
|
{
|
|
/*
|
|
* Suggests that __gntab_unmap_common failed early and so
|
|
@@ -1287,7 +1284,7 @@ __gnttab_unmap_common_complete(struct gnttab_unmap_common *op)
|
|
|
|
pg = mfn_to_page(op->frame);
|
|
|
|
- if ( op->flags & GNTMAP_device_map )
|
|
+ if ( op->dev_bus_addr && (op->flags & GNTMAP_device_map) )
|
|
{
|
|
if ( !is_iomem_page(act->frame) )
|
|
{
|
|
@@ -1358,6 +1355,7 @@ __gnttab_unmap_grant_ref(
|
|
/* Intialise these in case common contains old state */
|
|
common->new_addr = 0;
|
|
common->rd = NULL;
|
|
+ common->frame = 0;
|
|
|
|
__gnttab_unmap_common(common);
|
|
op->status = common->status;
|
|
@@ -1422,6 +1420,7 @@ __gnttab_unmap_and_replace(
|
|
/* Intialise these in case common contains old state */
|
|
common->dev_bus_addr = 0;
|
|
common->rd = NULL;
|
|
+ common->frame = 0;
|
|
|
|
__gnttab_unmap_common(common);
|
|
op->status = common->status;
|
|
--
|
|
2.1.4
|
|
|