[Xen-changelog] [xen stable-4.4] Limit XEN_DOMCTL_memory_mapping hypercall to only process up to 64 GFNs (or less)

[patchbot@xxxxxxx]

commit 518ae14973a44228fd7158c3d70270df6ed90033
Author:     Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
AuthorDate: Wed Nov 19 12:57:11 2014 -0500
Commit:     Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Wed Apr 1 10:14:34 2015 +0100

    Limit XEN_DOMCTL_memory_mapping hypercall to only process up to 64 GFNs (or 
less)
    
    Said hypercall for large BARs can take quite a while. As such
    we can require that the hypercall MUST break up the request
    in smaller values.
    
    Another approach is to add preemption to it - whether we do the
    preemption using hypercall_create_continuation or returning
    EAGAIN to userspace (and have it re-invocate the call) - either
    way the issue we cannot easily solve is that in 'map_mmio_regions'
    if we encounter an error we MUST call 'unmap_mmio_regions' for the
    whole BAR region.
    
    Since the preemption would re-use input fields such as nr_mfns,
    first_gfn, first_mfn - we would lose the original values -
    and only undo what was done in the current round (i.e. ignoring
    anything that was done prior to earlier preemptions).
    
    Unless we re-used the return value as 'EAGAIN|nr_mfns_done<<10' but
    that puts a limit (since the return value is a long) on the amount
    of nr_mfns that can provided.
    
    This patch sidesteps this problem by:
     - Setting an hard limit of nr_mfns having to be 64 or less.
     - Toolstack adjusts correspondingly to the nr_mfn limit.
     - If the there is an error when adding the toolstack will call the
       remove operation to remove the whole region.
    
    The need to break this hypercall down is for large BARs can take
    more than the guest (initial domain usually) time-slice. This has
    the negative result in that the guest is locked out for a long
    duration and is unable to act on any pending events.
    
    We also augment the code to return zero if nr_mfns instead
    of trying to the hypercall.
    
    This is XSA-125 / CVE-2015-2752.
    
    Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
    Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 tools/libxc/xc_domain.c     |   55 +++++++++++++++++++++++++++++++++++++++----
 xen/arch/x86/domctl.c       |    5 ++++
 xen/include/public/domctl.h |    1 +
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 369c3f3..40ca771 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1641,6 +1641,13 @@ failed:
     return -1;
 }
 
+#ifndef min
+#define min(X, Y) ({                             \
+            const typeof (X) _x = (X);           \
+            const typeof (Y) _y = (Y);           \
+            (void) (&_x == &_y);                 \
+            (_x < _y) ? _x : _y; })
+#endif
 int xc_domain_memory_mapping(
     xc_interface *xch,
     uint32_t domid,
@@ -1650,17 +1657,55 @@ int xc_domain_memory_mapping(
     uint32_t add_mapping)
 {
     DECLARE_DOMCTL;
+    int ret = 0, err;
+    unsigned long done = 0, nr, max_batch_sz;
+
+    if ( !nr_mfns )
+        return 0;
 
     domctl.cmd = XEN_DOMCTL_memory_mapping;
     domctl.domain = domid;
-    domctl.u.memory_mapping.first_gfn = first_gfn;
-    domctl.u.memory_mapping.first_mfn = first_mfn;
-    domctl.u.memory_mapping.nr_mfns = nr_mfns;
     domctl.u.memory_mapping.add_mapping = add_mapping;
+    max_batch_sz = nr_mfns;
+    do
+    {
+        nr = min(nr_mfns - done, max_batch_sz);
+        domctl.u.memory_mapping.nr_mfns = nr;
+        domctl.u.memory_mapping.first_gfn = first_gfn + done;
+        domctl.u.memory_mapping.first_mfn = first_mfn + done;
+        err = do_domctl(xch, &domctl);
+        if ( err && errno == E2BIG )
+        {
+            if ( max_batch_sz <= 1 )
+                break;
+            max_batch_sz >>= 1;
+            continue;
+        }
+        /* Save the first error... */
+        if ( !ret )
+            ret = err;
+        /* .. and ignore the rest of them when removing. */
+        if ( err && add_mapping != DPCI_REMOVE_MAPPING )
+            break;
 
-    return do_domctl(xch, &domctl);
-}
+        done += nr;
+    } while ( done < nr_mfns );
 
+    /*
+     * Undo what we have done unless unmapping, by unmapping the entire region.
+     * Errors here are ignored.
+     */
+    if ( ret && add_mapping != DPCI_REMOVE_MAPPING )
+        xc_domain_memory_mapping(xch, domid, first_gfn, first_mfn, nr_mfns,
+                                 DPCI_REMOVE_MAPPING);
+
+    /* We might get E2BIG so many times that we never advance. */
+    if ( !done && !ret )
+        ret = -1;
+
+    return ret;
+}
+#undef min
 int xc_domain_ioport_mapping(
     xc_interface *xch,
     uint32_t domid,
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index f5e9e2c..ea76a4f 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -655,6 +655,11 @@ long arch_do_domctl(
              (gfn + nr_mfns - 1) < gfn ) /* wrap? */
             break;
 
+        ret = -E2BIG;
+        /* Must break hypercall up as this could take a while. */
+        if ( nr_mfns > 64 )
+            break;
+
         ret = -EPERM;
         if ( !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
             break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 7e0b517..a151b29 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -519,6 +519,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_bind_pt_irq_t);
 
 
 /* Bind machine I/O address range -> HVM address range. */
+/* If this returns -E2BIG lower nr_mfns value. */
 /* XEN_DOMCTL_memory_mapping */
 #define DPCI_ADD_MAPPING         1
 #define DPCI_REMOVE_MAPPING      0
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.4

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog