[xen stable-4.6] p2m: Check return value of p2m_set_entry() when decreasing reservation

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[xen stable-4.6] p2m: Check return value of p2m_set_entry() when decreasing reservation

patchbot
commit 47d3e739e0a1daf94b102b027fa3425cbdff2e60
Author:     George Dunlap <[hidden email]>
AuthorDate: Tue Nov 28 13:49:22 2017 +0100
Commit:     Jan Beulich <[hidden email]>
CommitDate: Tue Nov 28 13:49:22 2017 +0100

    p2m: Check return value of p2m_set_entry() when decreasing reservation
   
    If the entire range specified to p2m_pod_decrease_reservation() is marked
    populate-on-demand, then it will make a single p2m_set_entry() call,
    reducing its PoD entry count.
   
    Unfortunately, in the right circumstances, this p2m_set_entry() call
    may fail.  It that case, repeated calls to decrease_reservation() may
    cause p2m->pod.entry_count to fall below zero, potentially tripping
    over BUG_ON()s to the contrary.
   
    Instead, check to see if the entry succeeded, and return false if not.
    The caller will then call guest_remove_page() on the gfns, which will
    return -EINVAL upon finding no valid memory there to return.
   
    Unfortunately if the order > 0, the entry may have partially changed.
    A domain_crash() is probably the safest thing in that case.
   
    Other p2m_set_entry() calls in the same function should be fine,
    because they are writing the entry at its current order.  Nonetheless,
    check the return value and crash if our assumption turns otu to be
    wrong.
   
    This is part of XSA-247.
   
    Signed-off-by: George Dunlap <[hidden email]>
    Reviewed-by: Jan Beulich <[hidden email]>
    master commit: a3d64de8e86f5812917d2d0af28298f80debdf9a
    master date: 2017-11-28 13:13:26 +0100
---
 xen/arch/x86/mm/p2m-pod.c | 42 +++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index b1f0abe..9324f16 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -559,11 +559,23 @@ recount:
 
     if ( !nonpod )
     {
-        /* All PoD: Mark the whole region invalid and tell caller
-         * we're done. */
-        p2m_set_entry(p2m, gpfn, _mfn(INVALID_MFN), order, p2m_invalid,
-                      p2m->default_access);
-        p2m->pod.entry_count-=(1<<order);
+        /*
+         * All PoD: Mark the whole region invalid and tell caller
+         * we're done.
+         */
+        if ( p2m_set_entry(p2m, gpfn, _mfn(INVALID_MFN), order, p2m_invalid,
+                           p2m->default_access) )
+        {
+            /*
+             * If this fails, we can't tell how much of the range was changed.
+             * Best to crash the domain unless we're sure a partial change is
+             * impossible.
+             */
+            if ( order != 0 )
+                domain_crash(d);
+            goto out_unlock;
+        }
+        p2m->pod.entry_count -= 1UL << order;
         BUG_ON(p2m->pod.entry_count < 0);
         ret = 1;
         goto out_entry_check;
@@ -595,8 +607,14 @@ recount:
         mfn = p2m->get_entry(p2m, gpfn + i, &t, &a, 0, NULL, NULL);
         if ( t == p2m_populate_on_demand )
         {
-            p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid,
-                          p2m->default_access);
+            /* This shouldn't be able to fail */
+            if ( p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0,
+                               p2m_invalid, p2m->default_access) )
+            {
+                ASSERT_UNREACHABLE();
+                domain_crash(d);
+                goto out_unlock;
+            }
             p2m->pod.entry_count--;
             BUG_ON(p2m->pod.entry_count < 0);
             pod--;
@@ -609,8 +627,14 @@ recount:
 
             page = mfn_to_page(mfn);
 
-            p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0, p2m_invalid,
-                          p2m->default_access);
+            /* This shouldn't be able to fail */
+            if ( p2m_set_entry(p2m, gpfn + i, _mfn(INVALID_MFN), 0,
+                               p2m_invalid, p2m->default_access) )
+            {
+                ASSERT_UNREACHABLE();
+                domain_crash(d);
+                goto out_unlock;
+            }
             set_gpfn_from_mfn(mfn_x(mfn), INVALID_M2P_ENTRY);
 
             p2m_pod_cache_add(p2m, page, 0);
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.6

_______________________________________________
Xen-changelog mailing list
[hidden email]
https://lists.xenproject.org/xen-changelog