File xsa224-1.patch of Package xen.11298
From: George Dunlap <george.dunlap@citrix.com>
Subject: 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>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -899,8 +899,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);
@@ -957,16 +955,14 @@ __gnttab_unmap_common(
op->ref = map->ref;
act = &active_entry(rgt, map->ref);
- 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(unmap_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;
}
@@ -1057,7 +1053,8 @@ __gnttab_unmap_common_complete(struct gn
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
@@ -1068,7 +1065,7 @@ __gnttab_unmap_common_complete(struct gn
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) )
{
@@ -1136,6 +1133,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;
@@ -1200,6 +1198,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;