[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |