Differential D16416 Diff 46059 head/emulators/xen-kernel47/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch
Changeset View
Changeset View
Standalone View
Standalone View
head/emulators/xen-kernel47/files/0001-gnttab-Fix-handling-of-dev_bus_addr-during-unmap.patch
Property | Old Value | New Value |
---|---|---|
fbsd:nokeywords | null | yes \ No newline at end of property |
svn:eol-style | null | native \ No newline at end of property |
svn:mime-type | null | text/plain \ No newline at end of property |
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 | |||||