[Xen-changelog] [xen master] xen/kexec: use hypercall_create_continuation to protect KEXEC ops

[patchbot@xxxxxxx]

commit 4f0b707feb673b2497ee2fa2454eaf4bbafaea2b
Author:     Eric DeVolder <eric.devolder@xxxxxxxxxx>
AuthorDate: Wed Apr 19 16:01:48 2017 -0500
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Thu Apr 20 18:11:36 2017 +0100

    xen/kexec: use hypercall_create_continuation to protect KEXEC ops
    
    When we concurrently try to unload and load crash
    images we eventually get:
    
     Xen call trace:
        [<ffff82d08018b04f>] machine_kexec_add_page+0x3a0/0x3fa
        [<ffff82d08018b184>] machine_kexec_load+0xdb/0x107
        [<ffff82d080116e8d>] kexec.c#kexec_load_slot+0x11/0x42
        [<ffff82d08011724f>] kexec.c#kexec_load+0x119/0x150
        [<ffff82d080117c1e>] kexec.c#do_kexec_op_internal+0xab/0xcf
        [<ffff82d080117c60>] do_kexec_op+0xe/0x1e
        [<ffff82d08025c620>] pv_hypercall+0x20a/0x44a
        [<ffff82d080260116>] cpufreq.c#test_all_events+0/0x30
    
     Pagetable walk from ffff820040088320:
      L4[0x104] = 00000002979d1063 ffffffffffffffff
      L3[0x001] = 00000002979d0063 ffffffffffffffff
      L2[0x000] = 00000002979c7063 ffffffffffffffff
      L1[0x088] = 80037a91ede97063 ffffffffffffffff
    
    The interesting thing is that the page bits (063) look legit.
    
    The operation on which we blow up is us trying to write
    in the L1 and finding that the L2 entry points to some
    bizzare MFN. It stinks of a race, and it looks like
    the issue is due to no concurrency locks when dealing
    with the crash kernel space.
    
    Specifically we concurrently call kimage_alloc_crash_control_page
    which iterates over the kexec_crash_area.start -> kexec_crash_area.size
    and once found:
    
      if ( page )
      {
          image->next_crash_page = hole_end;
          clear_domain_page(_mfn(page_to_mfn(page)));
      }
    
    clears. Since the parameters of what MFN to use are provided
    by the callers (and the area to search is bounded) the the 'page'
    is probably the same. So #1 we concurrently clear the
    'control_code_page'.
    
    The next step is us passing this 'control_code_page' to
    machine_kexec_add_page. This function requires the MFNs:
    page_to_maddr(image->control_code_page).
    
    And this would always return the same virtual address, as
    the MFN of the control_code_page is inside of the
    kexec_crash_area.start -> kexec_crash_area.size area.
    
    Then machine_kexec_add_page updates the L1 .. which can be done
    concurrently and on subsequent calls we mangle it up.
    
    This is all a theory at this time, but testing reveals
    that adding the hypercall_create_continuation() at the
    kexec hypercall fixes the crash.
    
    NOTE: This patch follows 5c5216 (kexec: clear kexec_image slot
    when unloading kexec image) to prevent crashes during
    simultaneous load/unloads.
    
    NOTE: Consideration was given to using the existing flag
    KEXEC_FLAG_IN_PROGRESS to denote a kexec hypercall in
    progress. This, however, overloads the original intent of
    the flag which is to denote that we are about-to/have made
    the jump to the crash path. The overloading would lead to
    failures in existing checks on this flag as the flag would
    always be set at the top level in do_kexec_op_internal().
    For this reason, the new flag KEXEC_FLAG_HC_IN_PROGRESS
    was introduced.
    
    While at it, fixed the #define mismatched spacing
    
    Signed-off-by: Eric DeVolder <eric.devolder@xxxxxxxxxx>
    Reviewed-by: Bhavesh Davda <bhavesh.davda@xxxxxxxxxx>
    Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
    Release-acked-by: Julien Grall <julien.grall@xxxxxxx>
---
 xen/common/kexec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 072cc8e..d6568e3 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -53,6 +53,7 @@ static struct kexec_image *kexec_image[KEXEC_IMAGE_NR];
 #define KEXEC_FLAG_DEFAULT_POS   (KEXEC_IMAGE_NR + 0)
 #define KEXEC_FLAG_CRASH_POS     (KEXEC_IMAGE_NR + 1)
 #define KEXEC_FLAG_IN_PROGRESS   (KEXEC_IMAGE_NR + 2)
+#define KEXEC_FLAG_IN_HYPERCALL  (KEXEC_IMAGE_NR + 3)
 
 static unsigned long kexec_flags = 0; /* the lowest bits are for 
KEXEC_IMAGE... */
 
@@ -1193,6 +1194,9 @@ static int do_kexec_op_internal(unsigned long op,
     if ( ret )
         return ret;
 
+    if ( test_and_set_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) )
+        return hypercall_create_continuation(__HYPERVISOR_kexec_op, "lh", op, 
uarg);
+
     switch ( op )
     {
     case KEXEC_CMD_kexec_get_range:
@@ -1227,6 +1231,8 @@ static int do_kexec_op_internal(unsigned long op,
         break;
     }
 
+    clear_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags);
+
     return ret;
 }
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

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