[Xen-changelog] [xen stable-4.8] gnttab: fix handling of dev_bus_addr during unmap

[patchbot@xxxxxxx]

commit 1959b49f350f5f3e78525720ead169f262ff8596
Author:     George Dunlap <george.dunlap@xxxxxxxxxx>
AuthorDate: Tue Jun 20 16:05:14 2017 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Jun 20 16:05:14 2017 +0200

    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@xxxxxxxx>
    Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    master commit: 8fdfcb2b6bcd074776560e76843815f124d587f1
    master date: 2017-06-20 14:45:33 +0200
---
 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 9fb879f..ebcf2d1 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;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.8

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog