[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 13/13] x86/domctl: Implement XEN_DOMCTL_set_cpumsr_policy



From: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>

This hypercall allows the toolstack to present one combined CPUID and MSR
policy for a domain, which can be audited in one go by Xen, which is necessary
for correctness of the auditing.

A stub x86_policies_are_compatible() function is introduced, although at
present it will always fail the hypercall.

The hypercall ABI allows for update of individual CPUID or MSR entries, so
begins by duplicating the existing policy (for which a helper is introduced),
merging the toolstack data, then checking compatibility of the result.

The system PV/HVM max policy is used for the compatiblity check.

Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
CC: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>

One awkard corner case is re-deserialising of the vcpu msrs.  The correct fix
would be to allocate a buffer, copy the MSRs list, then deserialise from that,
but trips the bounds checks in the copy_from_guest() helpers.  The compat XLAT
are would work, but would require that we allocate it even for 64bit PV
guests.
---
 tools/libxc/Makefile                |  2 +-
 tools/libxc/include/xenctrl.h       |  5 +++
 tools/libxc/xc_cpuid_x86.c          | 49 +++++++++++++++++++++
 xen/arch/x86/domctl.c               | 87 +++++++++++++++++++++++++++++++++++++
 xen/common/libx86/Makefile          |  1 +
 xen/common/libx86/policies.c        | 19 ++++++++
 xen/include/public/domctl.h         |  7 +++
 xen/include/xen/libx86/policies.h   | 10 +++++
 xen/include/xen/xmalloc.h           |  7 +++
 xen/xsm/flask/hooks.c               |  1 +
 xen/xsm/flask/policy/access_vectors |  1 +
 11 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 xen/common/libx86/policies.c

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index 407a9d1..5698fb0 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -83,7 +83,7 @@ $(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += 
-Wno-pointer-sign
 # Add libx86 to the build
 vpath %.c ../../xen/common/libx86
 
-GUEST_SRCS-$(CONFIG_X86)     += cpuid.c msr.c
+GUEST_SRCS-$(CONFIG_X86)     += cpuid.c msr.c policies.c
 
 # new domain builder
 GUEST_SRCS-y                 += xc_dom_core.c xc_dom_boot.c
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 15d2b92..653dcde 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2544,6 +2544,11 @@ int xc_get_system_cpumsr_policy(xc_interface *xch, 
uint32_t index,
 int xc_get_domain_cpumsr_policy(xc_interface *xch, uint32_t domid,
                                 uint32_t *nr_leaves, xen_cpuid_leaf_t *leaves,
                                 uint32_t *nr_msrs, xen_msr_entry_t *msrs);
+int xc_set_domain_cpumsr_policy(xc_interface *xch, uint32_t domid,
+                                uint32_t nr_leaves, xen_cpuid_leaf_t *leaves,
+                                uint32_t nr_msrs, xen_msr_entry_t *msrs,
+                                uint32_t *err_leaf_p, uint32_t *err_subleaf_p,
+                                uint32_t *err_msr_idx_p);
 
 uint32_t xc_get_cpu_featureset_size(void);
 
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index efbac77..ba0cf77 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -233,6 +233,55 @@ int xc_get_domain_cpumsr_policy(xc_interface *xch, 
uint32_t domid,
     return ret;
 }
 
+int xc_set_domain_cpumsr_policy(xc_interface *xch, uint32_t domid,
+                                uint32_t nr_leaves, xen_cpuid_leaf_t *leaves,
+                                uint32_t nr_msrs, xen_msr_entry_t *msrs,
+                                uint32_t *err_leaf_p, uint32_t *err_subleaf_p,
+                                uint32_t *err_msr_idx_p)
+{
+    DECLARE_DOMCTL;
+    DECLARE_HYPERCALL_BOUNCE(leaves,
+                             nr_leaves * sizeof(*leaves),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+    DECLARE_HYPERCALL_BOUNCE(msrs,
+                             nr_msrs * sizeof(*msrs),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+    int ret;
+
+    if ( xc_hypercall_bounce_pre(xch, leaves) )
+        return -1;
+
+    if ( xc_hypercall_bounce_pre(xch, msrs) )
+        return -1;
+
+    domctl.cmd = XEN_DOMCTL_set_cpumsr_policy;
+    domctl.domain = domid;
+    domctl.u.cpumsr_policy.nr_leaves = nr_leaves;
+    set_xen_guest_handle(domctl.u.cpumsr_policy.cpuid_policy, leaves);
+    domctl.u.cpumsr_policy.nr_msrs = nr_msrs;
+    set_xen_guest_handle(domctl.u.cpumsr_policy.msr_policy, msrs);
+    domctl.u.cpumsr_policy.err_leaf = ~0;
+    domctl.u.cpumsr_policy.err_subleaf = ~0;
+    domctl.u.cpumsr_policy.err_msr_idx = ~0;
+
+    ret = do_domctl(xch, &domctl);
+
+    xc_hypercall_bounce_post(xch, leaves);
+    xc_hypercall_bounce_post(xch, msrs);
+
+    if ( !ret )
+    {
+        if ( err_leaf_p )
+            *err_leaf_p = domctl.u.cpumsr_policy.err_leaf;
+        if ( err_subleaf_p )
+            *err_subleaf_p = domctl.u.cpumsr_policy.err_subleaf;
+        if ( err_msr_idx_p )
+            *err_msr_idx_p = domctl.u.cpumsr_policy.err_msr_idx;
+    }
+
+    return ret;
+}
+
 struct cpuid_domain_info
 {
     enum
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 8b48349..46ee1bb 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -330,6 +330,71 @@ static int update_domain_cpuid_info(struct domain *d,
     return 0;
 }
 
+static int update_domain_cpumsr_policy(struct domain *d,
+                                       xen_domctl_cpumsr_policy_t *xdpc)
+{
+    struct policy_group new = {};
+    const struct policy_group *sys = is_pv_domain(d)
+        ? &system_policies[XEN_SYSCTL_cpumsr_policy_pv_max]
+        : &system_policies[XEN_SYSCTL_cpumsr_policy_hvm_max];
+    struct vcpu *v = d->vcpu[0];
+    int ret = -ENOMEM;
+
+    /* Initialise some help identifying auditing errors. */
+    xdpc->err_leaf = xdpc->err_subleaf = XEN_CPUID_NO_SUBLEAF;
+    xdpc->err_msr_idx = ~0;
+
+    /* Start with existing domain's policies */
+    if ( !(new.cp = xmemdup(d->arch.cpuid)) ||
+         !(new.dp = xmemdup(d->arch.msr)) ||
+         !(new.vp = xmemdup(v->arch.msr)) )
+        goto out;
+
+    /* Merge the toolstack provided data. */
+    if ( (ret = x86_cpuid_copy_from_buffer(
+              new.cp, xdpc->cpuid_policy, xdpc->nr_leaves,
+              &xdpc->err_leaf, &xdpc->err_subleaf)) )
+        goto out;
+
+    if ( (ret = x86_msr_copy_from_buffer(
+              new.dp, new.vp,
+              xdpc->msr_policy, xdpc->nr_msrs, &xdpc->err_msr_idx)) )
+        goto out;
+
+    /* Audit the combined dataset. */
+    ret = x86_policies_are_compatible(sys, &new);
+    if ( ret )
+        goto out;
+
+    /*
+     * Audit was successful.  Replace existing policies, leaving the old
+     * policies to be freed.
+     */
+    SWAP(new.cp, d->arch.cpuid);
+    SWAP(new.dp, d->arch.msr);
+    SWAP(new.vp, v->arch.msr);
+
+    /* Merge the (now audited) vCPU MSRs into every other msr_vcpu_policy. */
+    for ( ; v; v = v->next_in_list )
+    {
+        /* XXX - Figure out how to avoid a TOCTOU race here.  XLAT area? */
+        if ( (ret = x86_msr_copy_from_buffer(
+                  NULL, v->arch.msr, xdpc->msr_policy, xdpc->nr_msrs, NULL)) )
+        {
+            gprintk(XENLOG_ERR, "Error re-deserialising vCPU MSRs: %d\n", ret);
+            domain_crash(d);
+            goto out;
+        }
+    }
+
+ out:
+    xfree(new.cp);
+    xfree(new.dp);
+    xfree(new.vp);
+
+    return ret;
+}
+
 static int vcpu_set_vmce(struct vcpu *v,
                          const struct xen_domctl_ext_vcpucontext *evc)
 {
@@ -1570,6 +1635,28 @@ long arch_do_domctl(
         domain_unpause(d);
         break;
 
+    case XEN_DOMCTL_set_cpumsr_policy:
+        if ( d == currd ||       /* no domain_pause() */
+             d->max_vcpus == 0 ) /* No vcpus yet. */
+        {
+            ret = -EINVAL;
+            break;
+        }
+
+        domain_pause(d);
+
+        if ( d->creation_finished )
+            ret = -EEXIST; /* No changing once the domain is running. */
+        else
+        {
+            ret = update_domain_cpumsr_policy(d, &domctl->u.cpumsr_policy);
+            if ( !ret ) /* Copy domctl->u.cpumsr_policy.err_* to guest. */
+                copyback = true;
+        }
+
+        domain_unpause(d);
+        break;
+
     default:
         ret = iommu_do_domctl(domctl, d, u_domctl);
         break;
diff --git a/xen/common/libx86/Makefile b/xen/common/libx86/Makefile
index 2f9691e..90d64ac 100644
--- a/xen/common/libx86/Makefile
+++ b/xen/common/libx86/Makefile
@@ -1,2 +1,3 @@
 obj-y += cpuid.o
 obj-y += msr.o
+obj-y += policies.o
diff --git a/xen/common/libx86/policies.c b/xen/common/libx86/policies.c
new file mode 100644
index 0000000..80de150
--- /dev/null
+++ b/xen/common/libx86/policies.c
@@ -0,0 +1,19 @@
+#include "libx86-private.h"
+
+#include <xen/libx86/policies.h>
+
+int x86_policies_are_compatible(const struct policy_group *lhs,
+                                const struct policy_group *rhs)
+{
+    return -EOPNOTSUPP;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 1ca41bd..4bc4629 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -648,6 +648,12 @@ struct xen_domctl_cpumsr_policy {
                          * 'msr_domain_policy' */
     XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT: */
     XEN_GUEST_HANDLE_64(xen_msr_entry_t) msr_policy;    /* IN/OUT: */
+    uint32_t err_leaf, err_subleaf; /* OUT, set_policy only.  If not ~0,
+                                     * indicates the leaf/subleaf which
+                                     * auditing objected to. */
+    uint32_t err_msr_idx;           /* OUT, set_policy only.  If not ~0,
+                                     * indicates the MSR idx which
+                                     * auditing objected to. */
 };
 typedef struct xen_domctl_cpumsr_policy xen_domctl_cpumsr_policy_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_cpumsr_policy_t);
@@ -1189,6 +1195,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_set_gnttab_limits             80
 #define XEN_DOMCTL_vuart_op                      81
 #define XEN_DOMCTL_get_cpumsr_policy             82
+#define XEN_DOMCTL_set_cpumsr_policy             83
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
diff --git a/xen/include/xen/libx86/policies.h 
b/xen/include/xen/libx86/policies.h
index d6fa1bc..21e0a40 100644
--- a/xen/include/xen/libx86/policies.h
+++ b/xen/include/xen/libx86/policies.h
@@ -12,6 +12,16 @@ struct policy_group
     struct msr_vcpu_policy *vp;
 };
 
+/*
+ * Calculate whether two policies are compatible.
+ *
+ * For typical usage, lhs should be a system policy, and rhs should be a
+ * proposed guest policy.  This largely amounts to "is rhs a subset of lhs",
+ * but some of the checks are rather more complicated in practice.
+ */
+int x86_policies_are_compatible(const struct policy_group *lhs,
+                                const struct policy_group *rhs);
+
 #endif /* !XEN_LIBX86_POLICIES_H */
 
 /*
diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h
index cc2673d..ee5fe84 100644
--- a/xen/include/xen/xmalloc.h
+++ b/xen/include/xen/xmalloc.h
@@ -13,6 +13,13 @@
 #define xmalloc(_type) ((_type *)_xmalloc(sizeof(_type), __alignof__(_type)))
 #define xzalloc(_type) ((_type *)_xzalloc(sizeof(_type), __alignof__(_type)))
 
+/* Allocate space for a typed object and copy an existing instance. */
+#define xmemdup(ptr)                                    \
+    ({  typeof(*ptr) *n_ = xmalloc(typeof(*ptr));       \
+        if ( n_ )                                       \
+            memcpy(n_, ptr, sizeof(*ptr));              \
+        n_; })
+
 /* Allocate space for array of typed objects. */
 #define xmalloc_array(_type, _num) \
     ((_type *)_xmalloc_array(sizeof(_type), __alignof__(_type), _num))
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 245fcfd..4ad87c7 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -718,6 +718,7 @@ static int flask_domctl(struct domain *d, int cmd)
 
     case XEN_DOMCTL_set_cpuid:
     case XEN_DOMCTL_get_cpumsr_policy:
+    case XEN_DOMCTL_set_cpumsr_policy:
         return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SET_CPUID);
 
     case XEN_DOMCTL_gettscinfo:
diff --git a/xen/xsm/flask/policy/access_vectors 
b/xen/xsm/flask/policy/access_vectors
index c92bea3..d76dc3f 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -214,6 +214,7 @@ class domain2
     set_as_target
 # XEN_DOMCTL_set_cpuid
 # XEN_DOMCTL_get_cpumsr_policy
+# XEN_DOMCTL_set_cpumsr_policy
     set_cpuid
 # XEN_DOMCTL_gettscinfo
     gettsc
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.