[Xen-changelog] [xen staging] libxc/restore: Fix data auditing in handle_x86_pv_info()

[patchbot@xxxxxxx]

commit aafae0e800e9936b9eb6566e5fcdbe823625a7d1
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Dec 18 20:17:42 2019 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Fri Dec 20 12:11:29 2019 +0000

    libxc/restore: Fix data auditing in handle_x86_pv_info()
    
    handle_x86_pv_info() has a subtle bug.  It uses an 'else if' chain with a
    clause in the middle which doesn't exit unconditionally.  In practice, this
    means that when restoring a 32bit PV guest, later sanity checks are skipped.
    
    Rework the logic a little to be simpler.  There are exactly two valid
    combinations of fields in X86_PV_INFO, so factor this out and check them all
    in one go, before making adjustments to the current domain.
    
    Once adjustments have been completed successfully, sanity check the result
    against the X86_PV_INFO settings in one go, rather than piece-wise.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxc/xc_sr_restore_x86_pv.c | 69 ++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/tools/libxc/xc_sr_restore_x86_pv.c 
b/tools/libxc/xc_sr_restore_x86_pv.c
index a2dbf85157..9e9ff32d47 100644
--- a/tools/libxc/xc_sr_restore_x86_pv.c
+++ b/tools/libxc/xc_sr_restore_x86_pv.c
@@ -582,6 +582,21 @@ static int update_guest_p2m(struct xc_sr_context *ctx)
 }
 
 /*
+ * The valid width/pt_levels values in X86_PV_INFO are inextricably linked.
+ * Cross-check the legitimate combinations.
+ */
+static bool valid_x86_pv_info_combination(
+    const struct xc_sr_rec_x86_pv_info *info)
+{
+    switch ( info->guest_width )
+    {
+    case 4:  return info->pt_levels == 3;
+    case 8:  return info->pt_levels == 4;
+    default: return false;
+    }
+}
+
+/*
  * Process an X86_PV_INFO record.
  */
 static int handle_x86_pv_info(struct xc_sr_context *ctx,
@@ -602,29 +617,31 @@ static int handle_x86_pv_info(struct xc_sr_context *ctx,
               rec->length, sizeof(*info));
         return -1;
     }
-    else if ( info->guest_width != 4 &&
-              info->guest_width != 8 )
+
+    if ( !valid_x86_pv_info_combination(info) )
     {
-        ERROR("Unexpected guest width %u, Expected 4 or 8",
-              info->guest_width);
+        ERROR("Invalid X86_PV_INFO combination: width %u, pt_levels %u",
+              info->guest_width, info->pt_levels);
         return -1;
     }
-    else if ( info->guest_width != ctx->x86_pv.width )
+
+    /*
+     * PV domains default to native width.  For an incomming compat domain, we
+     * will typically be the first entity to inform Xen.
+     */
+    if ( info->guest_width != ctx->x86_pv.width )
     {
-        int rc;
-        struct xen_domctl domctl;
-
-        /* Try to set address size, domain is always created 64 bit. */
-        memset(&domctl, 0, sizeof(domctl));
-        domctl.domain = ctx->domid;
-        domctl.cmd    = XEN_DOMCTL_set_address_size;
-        domctl.u.address_size.size = info->guest_width * 8;
-        rc = do_domctl(xch, &domctl);
+        struct xen_domctl domctl = {
+            .domain = ctx->domid,
+            .cmd    = XEN_DOMCTL_set_address_size,
+            .u.address_size.size = info->guest_width * 8,
+        };
+        int rc = do_domctl(xch, &domctl);
+
         if ( rc != 0 )
         {
-            ERROR("Width of guest in stream (%u"
-                  " bits) differs with existing domain (%u bits)",
-                  info->guest_width * 8, ctx->x86_pv.width * 8);
+            ERROR("Failed to update d%d address size to %u",
+                  ctx->domid, info->guest_width * 8);
             return -1;
         }
 
@@ -636,18 +653,14 @@ static int handle_x86_pv_info(struct xc_sr_context *ctx,
             return -1;
         }
     }
-    else if ( info->pt_levels != 3 &&
-              info->pt_levels != 4 )
-    {
-        ERROR("Unexpected guest levels %u, Expected 3 or 4",
-              info->pt_levels);
-        return -1;
-    }
-    else if ( info->pt_levels != ctx->x86_pv.levels )
+
+    /* Sanity check (possibly new) domain settings. */
+    if ( (info->guest_width != ctx->x86_pv.width) ||
+         (info->pt_levels   != ctx->x86_pv.levels) )
     {
-        ERROR("Levels of guest in stream (%u"
-              ") differs with existing domain (%u)",
-              info->pt_levels, ctx->x86_pv.levels);
+        ERROR("X86_PV_INFO width/pt_levels settings %u/%u mismatch with d%d 
%u/%u",
+              info->guest_width, info->pt_levels, ctx->domid,
+              ctx->x86_pv.width, ctx->x86_pv.levels);
         return -1;
     }
 
--
generated by git-patchbot for /home/xen/git/xen.git#staging

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