[Xen-changelog] [xen staging-4.10] tools/libxc: Fix issues with libxc and Xen having different featureset lengths

[patchbot@xxxxxxx]

commit 617a1e7829fe1808481b721edaf1c8e2ff4ed75d
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Thu Nov 29 18:10:38 2018 +0000
Commit:     Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
CommitDate: Fri May 17 10:46:16 2019 +0100

    tools/libxc: Fix issues with libxc and Xen having different featureset 
lengths
    
    In almost all cases, Xen and libxc will agree on the featureset length,
    because they are built from the same source.
    
    However, there are circumstances (e.g. security hotfixes) where the 
featureset
    gets longer and dom0 will, after installing updates, be running with an old
    Xen but new libxc.  Despite writing the code with this scenario in mind, 
there
    were some bugs.
    
    First, xen-cpuid's get_featureset() erroneously allocates a buffer based on
    Xen's featureset length, but records libxc's length, which may be longer.
    
    In this situation, the hypercall bounce buffer code reads/writes the 
recorded
    length, which is beyond the end of the allocated object, and a later free()
    encounters corrupt heap metadata.  Fix this by recording the same length 
that
    we allocate.
    
    Secondly, get_cpuid_domain_info() has a related bug when the passed-in
    featureset is a different length to libxc's.
    
    A large amount of the libxc cpuid functionality depends on info->featureset
    being as long as expected, and it is allocated appropriately.  However, in 
the
    case that a shorter external featureset is passed in, the logic to check for
    trailing nonzero bits may read off the end of it.  Rework the logic to use 
the
    correct upper bound.
    
    In addition, leave a comment next to the fields in struct cpuid_domain_info
    explaining the relationship between the various lengths, and how to cope 
with
    different lengths.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
    (cherry picked from commit c393b64dcee6684da25257b033148740cb6d7ff0)
    (cherry picked from commit 5b97821919c399bbcda95edef6926ae7d29c8817)
---
 tools/libxc/xc_cpuid_x86.c | 23 +++++++++++++++++++++--
 tools/misc/xen-cpuid.c     |  2 +-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 21537f06f1..e62f920797 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -169,6 +169,18 @@ struct cpuid_domain_info
     bool hvm;
     uint64_t xfeature_mask;
 
+    /*
+     * Careful with featureset lengths.
+     *
+     * Code in this file requires featureset to have at least
+     * xc_get_cpu_featureset_size() entries.  This is a libxc compiletime
+     * constant.
+     *
+     * The featureset length used by the hypervisor may be different.  If the
+     * hypervisor version is longer, XEN_SYSCTL_get_cpu_featureset will fail
+     * with -ENOBUFS, and libxc really does need rebuilding.  If the
+     * hypervisor version is shorter, it is safe to zero-extend.
+     */
     uint32_t *featureset;
     unsigned int nr_features;
 
@@ -239,11 +251,18 @@ static int get_cpuid_domain_info(xc_interface *xch, 
uint32_t domid,
 
     if ( featureset )
     {
+        /*
+         * The user supplied featureset may be shorter or longer than
+         * host_nr_features.  Shorter is fine, and we will zero-extend.
+         * Longer is fine, so long as it only padded with zeros.
+         */
+        unsigned int fslen = min(host_nr_features, nr_features);
+
         memcpy(info->featureset, featureset,
-               min(host_nr_features, nr_features) * sizeof(*info->featureset));
+               fslen * sizeof(*info->featureset));
 
         /* Check for truncated set bits. */
-        for ( i = nr_features; i < host_nr_features; ++i )
+        for ( i = fslen; i < nr_features; ++i )
             if ( featureset[i] != 0 )
                 return -EOPNOTSUPP;
     }
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 570424de75..89d50e048c 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -249,7 +249,7 @@ static void get_featureset(xc_interface *xch, unsigned int 
idx)
 {
     struct fsinfo *f = &featuresets[idx];
 
-    f->len = xc_get_cpu_featureset_size();
+    f->len = nr_features;
     f->fs = calloc(nr_features, sizeof(*f->fs));
 
     if ( !f->fs )
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.10

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