| Message ID | 1458064616-23101-4-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 15.03.16 at 18:56, <konrad.wilk@oracle.com> wrote: > @@ -223,12 +224,15 @@ void __init do_initcalls(void) > /* > * Simple hypercalls. > */ > - > DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) Please retain the blank line, as it relates to more than just this one function. > { > + bool_t deny = !!xsm_xen_version(XSM_OTHER, cmd); > + > switch ( cmd ) > { > case XENVER_version: > + if ( deny ) > + return 0; > return (xen_major_version() << 16) | xen_minor_version(); To be honest, I'm now rather uncertain about this one: If a guest can't figure out the hypervisor version, how would it be able to adjust its behavior accordingly (e.g. use deprecated hypercalls as needed)? IOW, other than for most/all other stuff here (the get-features and platform-parameters sub-ops may be considered similar to this one, see also below), I don't think allowing the "permitted" default to be overridden makes sense here. > @@ -274,6 +279,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > .virt_start = HYPERVISOR_VIRT_START > }; > > + if ( deny ) > + params.virt_start = 0; Guests may (validly imo) assume to get a valid address here. If you mean to not expose the non-constant address in the compat mode case, I could accept that. But you would then need to set the ABI mandated __HYPERVISOR_COMPAT_VIRT_START (and retain the constant value in the non-compat case). Our old 32-bit PV guests would crash extremely early on boot if they got back zero here (that's for 2.6.30 and later, and I think both you and Citrix had derived some of their kernels from our 2.6.32 based one). > @@ -302,6 +310,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > switch ( fi.submap_idx ) > { > case 0: > + if ( deny ) > + break; I think if to be put here at all, this should go ahead of the switch(), so that guests wouldn't be able to guess from the valid index values which features may be available. And of course you should clear fi.submap if you deny access, instead of leaving in it what has been there before. > case XENVER_guest_handle: > - if ( copy_to_guest(arg, current->domain->handle, > - ARRAY_SIZE(current->domain->handle)) ) > + { > + xen_domain_handle_t hdl; > + ssize_t len; > + > + if ( deny ) > + { > + len = sizeof(hdl); > + memset(&hdl, 0, len); > + } else > + len = ARRAY_SIZE(current->domain->handle); > + > + if ( copy_to_guest(arg, deny ? hdl : current->domain->handle, len ) ) > return -EFAULT; > return 0; What is this "len" handling here about? Aren't both the same type and hence size? Perhaps, if you feel unsure about that, simply add a respective BUILD_BUG_ON()? > --- a/xen/include/xen/version.h > +++ b/xen/include/xen/version.h > @@ -12,5 +12,5 @@ unsigned int xen_minor_version(void); > const char *xen_extra_version(void); > const char *xen_changeset(void); > const char *xen_banner(void); > - > +const char *xen_deny(void); > #endif /* __XEN_VERSION_H__ */ Please retain the blank line. Jan
On 03/15/2016 01:56 PM, Konrad Rzeszutek Wilk wrote: > All of XENVER_* have now an XSM check for their sub-ops. > > The subop for XENVER_commandline is now a priviliged operation. > To not break guests we still return an string - but it is > just '<denied>\0'. > > The rest: XENVER_[version|extraversion|capabilities| > parameters|get_features|page_size|guest_handle|changeset| > compile_info] behave as before - allowed by default for all > guests if using the XSM default policy or with the dummy one. > > The admin can choose to change the sub-ops to be denied > as they see fit. > > Also we add a local variable block. > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
On 15/03/16 17:56, Konrad Rzeszutek Wilk wrote: > All of XENVER_* have now an XSM check for their sub-ops. > > The subop for XENVER_commandline is now a priviliged operation. > To not break guests we still return an string - but it is > just '<denied>\0'. > > The rest: XENVER_[version|extraversion|capabilities| > parameters|get_features|page_size|guest_handle|changeset| > compile_info] behave as before - allowed by default for all > guests if using the XSM default policy or with the dummy one. > > The admin can choose to change the sub-ops to be denied > as they see fit. > > Also we add a local variable block. > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > --- > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > > v2: Do XSM check for all the XENVER_ ops. > v3: Add empty data conditions. > v4: Return <denied> for priv subops. > v5: Move extraversion from priv to normal. Drop the XSM check > for the non-priv subops. > v6: Add +1 for strlen(xen_deny()) to include NULL. Move changeset, > compile_info to non-priv subops. > v7: Remove the \0 on xen_deny() > v8: Add new XSM domain for xenver hypercall. Add all subops to it. > v9: Remove the extra line, Add Ack from Daniel > v10: Rename the XSM from xen_version_op to xsm_xen_version. > Prefix the types with 'xen' to distinguish it from another > hypercall performing similar operation. Removed Ack from Daniel > as it was so large. Add local variable block. > --- > tools/flask/policy/policy/modules/xen/xen.te | 15 ++++++++ > xen/common/kernel.c | 53 +++++++++++++++++++++------- > xen/common/version.c | 15 ++++++++ > xen/include/xen/version.h | 2 +- > xen/include/xsm/dummy.h | 22 ++++++++++++ > xen/include/xsm/xsm.h | 5 +++ > xen/xsm/dummy.c | 1 + > xen/xsm/flask/hooks.c | 43 ++++++++++++++++++++++ > xen/xsm/flask/policy/access_vectors | 28 +++++++++++++++ > xen/xsm/flask/policy/security_classes | 1 + > 10 files changed, 172 insertions(+), 13 deletions(-) > > diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te > index d35ae22..7e7400d 100644 > --- a/tools/flask/policy/policy/modules/xen/xen.te > +++ b/tools/flask/policy/policy/modules/xen/xen.te > @@ -73,6 +73,15 @@ allow dom0_t xen_t:xen2 { > pmu_ctrl > get_symbol > }; > + > +# Allow dom0 to use all XENVER_ subops > +# Note that dom0 is part of domain_type so this has duplicates. > +allow dom0_t xen_t:version { > + xen_version xen_extraversion xen_compile_info xen_capabilities > + xen_changeset xen_platform_parameters xen_get_features xen_pagesize > + xen_guest_handle xen_commandline > +}; > + > allow dom0_t xen_t:mmu memorymap; > > # Allow dom0 to use these domctls on itself. For domctls acting on other > @@ -137,6 +146,12 @@ if (guest_writeconsole) { > # pmu_ctrl is for) > allow domain_type xen_t:xen2 pmu_use; > > +# For normal guests all except XENVER_commandline > +allow domain_type xen_t:version { > + xen_version xen_extraversion xen_compile_info xen_capabilities > + xen_changeset xen_platform_parameters xen_get_features xen_pagesize > + xen_guest_handle > +}; > ############################################################################### > # > # Domain creation > diff --git a/xen/common/kernel.c b/xen/common/kernel.c > index 0618da2..2699ac0 100644 > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -13,6 +13,7 @@ > #include <xen/nmi.h> > #include <xen/guest_access.h> > #include <xen/hypercall.h> > +#include <xsm/xsm.h> > #include <asm/current.h> > #include <public/nmi.h> > #include <public/version.h> > @@ -223,12 +224,15 @@ void __init do_initcalls(void) > /* > * Simple hypercalls. > */ > - > DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > { > + bool_t deny = !!xsm_xen_version(XSM_OTHER, cmd); > + > switch ( cmd ) > { > case XENVER_version: > + if ( deny ) > + return 0; > return (xen_major_version() << 16) | xen_minor_version(); > > case XENVER_extraversion: > @@ -236,7 +240,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > xen_extraversion_t extraversion; > > memset(extraversion, 0, sizeof(extraversion)); > - safe_strcpy(extraversion, xen_extra_version()); > + safe_strcpy(extraversion, deny ? xen_deny() : xen_extra_version()); > if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) ) > return -EFAULT; > return 0; > @@ -247,10 +251,10 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > xen_compile_info_t info; > > memset(&info, 0, sizeof(info)); > - safe_strcpy(info.compiler, xen_compiler()); > - safe_strcpy(info.compile_by, xen_compile_by()); > - safe_strcpy(info.compile_domain, xen_compile_domain()); > - safe_strcpy(info.compile_date, xen_compile_date()); > + safe_strcpy(info.compiler, deny ? xen_deny() : xen_compiler()); > + safe_strcpy(info.compile_by, deny ? xen_deny() : xen_compile_by()); > + safe_strcpy(info.compile_domain, deny ? xen_deny() : xen_compile_domain()); > + safe_strcpy(info.compile_date, deny ? xen_deny() : xen_compile_date()); > if ( copy_to_guest(arg, &info, 1) ) > return -EFAULT; > return 0; > @@ -261,7 +265,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > xen_capabilities_info_t info; > > memset(info, 0, sizeof(info)); > - arch_get_xen_caps(&info); > + if ( !deny ) > + arch_get_xen_caps(&info); > > if ( copy_to_guest(arg, info, ARRAY_SIZE(info)) ) > return -EFAULT; > @@ -274,6 +279,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > .virt_start = HYPERVISOR_VIRT_START > }; > > + if ( deny ) > + params.virt_start = 0; > + > if ( copy_to_guest(arg, ¶ms, 1) ) > return -EFAULT; > return 0; > @@ -285,7 +293,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > xen_changeset_info_t chgset; > > memset(chgset, 0, sizeof(chgset)); > - safe_strcpy(chgset, xen_changeset()); > + safe_strcpy(chgset, deny ? xen_deny() : xen_changeset()); > if ( copy_to_guest(arg, chgset, ARRAY_SIZE(chgset)) ) > return -EFAULT; > return 0; > @@ -302,6 +310,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > switch ( fi.submap_idx ) > { > case 0: > + if ( deny ) > + break; > fi.submap = (1U << XENFEAT_memory_op_vnode_supported); > if ( VM_ASSIST(d, pae_extended_cr3) ) > fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb); > @@ -342,19 +352,38 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > } > > case XENVER_pagesize: > + if ( deny ) > + return 0; > return (!guest_handle_is_null(arg) ? -EINVAL : PAGE_SIZE); > > case XENVER_guest_handle: > - if ( copy_to_guest(arg, current->domain->handle, > - ARRAY_SIZE(current->domain->handle)) ) > + { > + xen_domain_handle_t hdl; > + ssize_t len; > + > + if ( deny ) > + { > + len = sizeof(hdl); > + memset(&hdl, 0, len); > + } else > + len = ARRAY_SIZE(current->domain->handle); > + > + if ( copy_to_guest(arg, deny ? hdl : current->domain->handle, len ) ) > return -EFAULT; > return 0; > - > + } > case XENVER_commandline: > - if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) ) > + { > + size_t len = ARRAY_SIZE(saved_cmdline); > + > + if ( deny ) > + len = strlen(xen_deny()) + 1; > + > + if ( copy_to_guest(arg, deny ? xen_deny() : saved_cmdline, len) ) > return -EFAULT; > return 0; > } > + } > > return -ENOSYS; > } > diff --git a/xen/common/version.c b/xen/common/version.c > index b152e27..fc9bf42 100644 > --- a/xen/common/version.c > +++ b/xen/common/version.c > @@ -55,3 +55,18 @@ const char *xen_banner(void) > { > return XEN_BANNER; > } > + > +const char *xen_deny(void) > +{ > + return "<denied>"; > +} > + > +/* > + * 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/xen/version.h b/xen/include/xen/version.h > index 81a3c7d..016a56c 100644 > --- a/xen/include/xen/version.h > +++ b/xen/include/xen/version.h > @@ -12,5 +12,5 @@ unsigned int xen_minor_version(void); > const char *xen_extra_version(void); > const char *xen_changeset(void); > const char *xen_banner(void); > - > +const char *xen_deny(void); > #endif /* __XEN_VERSION_H__ */ > diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h > index 1d13826..94b8855 100644 > --- a/xen/include/xsm/dummy.h > +++ b/xen/include/xsm/dummy.h > @@ -727,3 +727,25 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int > } > > #endif /* CONFIG_X86 */ > + > +#include <public/version.h> > +static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op) > +{ > + XSM_ASSERT_ACTION(XSM_OTHER); > + switch ( op ) > + { > + case XENVER_version: > + case XENVER_extraversion: > + case XENVER_compile_info: > + case XENVER_capabilities: > + case XENVER_changeset: > + case XENVER_platform_parameters: > + case XENVER_get_features: > + case XENVER_pagesize: > + case XENVER_guest_handle: > + /* These MUST always be accessible to any guest by default. */ > + return xsm_default_action(XSM_HOOK, current->domain, NULL); > + default: > + return xsm_default_action(XSM_PRIV, current->domain, NULL); > + } > +} > diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h > index 3afed70..db440f6 100644 > --- a/xen/include/xsm/xsm.h > +++ b/xen/include/xsm/xsm.h > @@ -193,6 +193,7 @@ struct xsm_operations { > int (*ioport_mapping) (struct domain *d, uint32_t s, uint32_t e, uint8_t allow); > int (*pmu_op) (struct domain *d, unsigned int op); > #endif > + int (*xen_version) (uint32_t cmd); > }; > > #ifdef CONFIG_XSM > @@ -731,6 +732,10 @@ static inline int xsm_pmu_op (xsm_default_t def, struct domain *d, unsigned int > > #endif /* CONFIG_X86 */ > > +static inline int xsm_xen_version (xsm_default_t def, uint32_t op) > +{ > + return xsm_ops->xen_version(op); > +} > #endif /* XSM_NO_WRAPPERS */ > > #ifdef CONFIG_MULTIBOOT > diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c > index 0f32636..9791ad4 100644 > --- a/xen/xsm/dummy.c > +++ b/xen/xsm/dummy.c > @@ -162,4 +162,5 @@ void xsm_fixup_ops (struct xsm_operations *ops) > set_to_dummy_if_null(ops, ioport_mapping); > set_to_dummy_if_null(ops, pmu_op); > #endif > + set_to_dummy_if_null(ops, xen_version); > } > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c > index 4813623..d1bef43 100644 > --- a/xen/xsm/flask/hooks.c > +++ b/xen/xsm/flask/hooks.c > @@ -26,6 +26,7 @@ > #include <public/xen.h> > #include <public/physdev.h> > #include <public/platform.h> > +#include <public/version.h> > > #include <public/xsm/flask_op.h> > > @@ -1620,6 +1621,47 @@ static int flask_pmu_op (struct domain *d, unsigned int op) > } > #endif /* CONFIG_X86 */ > > +static int flask_xen_version (uint32_t op) > +{ > + u32 dsid = domain_sid(current->domain); > + > + switch ( op ) > + { > + case XENVER_version: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_VERSION, NULL); > + case XENVER_extraversion: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_EXTRAVERSION, NULL); > + case XENVER_compile_info: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_COMPILE_INFO, NULL); > + case XENVER_capabilities: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_CAPABILITIES, NULL); > + case XENVER_changeset: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_CHANGESET, NULL); > + case XENVER_platform_parameters: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_PLATFORM_PARAMETERS, NULL); > + case XENVER_get_features: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_GET_FEATURES, NULL); > + case XENVER_pagesize: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_PAGESIZE, NULL); > + case XENVER_guest_handle: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_GUEST_HANDLE, NULL); > + case XENVER_commandline: > + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, > + VERSION__XEN_COMMANDLINE, NULL); > + default: > + return -EPERM; > + } > +} > + > long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op); > int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op); > > @@ -1758,6 +1800,7 @@ static struct xsm_operations flask_ops = { > .ioport_mapping = flask_ioport_mapping, > .pmu_op = flask_pmu_op, > #endif > + .xen_version = flask_xen_version, > }; > > static __init void flask_init(void) > diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors > index effb59f..628dd5c 100644 > --- a/xen/xsm/flask/policy/access_vectors > +++ b/xen/xsm/flask/policy/access_vectors > @@ -495,3 +495,31 @@ class security > # remove ocontext label definitions for resources > del_ocontext > } > + > +# Class version is used to describe the XENVER_ hypercall. > +# Each sub-ops is described here - in the default case all of them should > +# be allowed except the XENVER_commandline. > +# > +class version > +{ > +# Often called by PV kernels to force an callback. > + xen_version > +# Extra informations (-unstable). > + xen_extraversion > +# Compile information of the hypervisor. > + xen_compile_info > +# Such as "xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64". > + xen_capabilities > +# Such as the virtual address of where the hypervisor resides. > + xen_platform_parameters > +# Source code changeset. > + xen_changeset > +# The features the hypervisor supports. > + xen_get_features > +# Page size the hypervisor uses. > + xen_pagesize > +# An value that the control stack can choose. > + xen_guest_handle > +# Xen command line. > + xen_commandline > +} > diff --git a/xen/xsm/flask/policy/security_classes b/xen/xsm/flask/policy/security_classes > index ca191db..cde4e1a 100644 > --- a/xen/xsm/flask/policy/security_classes > +++ b/xen/xsm/flask/policy/security_classes > @@ -18,5 +18,6 @@ class shadow > class event > class grant > class security > +class version > > # FLASK > Can we have more meaningful name for XSM class. "version" doesn't seem to be informative enough to convey the message as to why we need it to be secure. (Is it a resource, or domain specific or event or...) My suggestion would be xenmetainfo or something more meaningful. Anshul
> >diff --git a/xen/xsm/flask/policy/security_classes b/xen/xsm/flask/policy/security_classes > >index ca191db..cde4e1a 100644 > >--- a/xen/xsm/flask/policy/security_classes > >+++ b/xen/xsm/flask/policy/security_classes > >@@ -18,5 +18,6 @@ class shadow > > class event > > class grant > > class security > >+class version > > > > # FLASK > > > Can we have more meaningful name for XSM class. "version" doesn't seem to be > informative enough to convey the message as to why we need it to be secure. > (Is it a resource, or domain specific or event or...) Heya! 1). Please trim your replies. 2). The patch is already in the tree. 3). If you prefer a different name - by all means please submit a patch for that. I am all up for it. > > My suggestion would be xenmetainfo or something more meaningful. /me blinks Metainfo? I am not sure how one is suppose to associate that with features that the hypervisor is exposing to guests. > > Anshul >
diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te index d35ae22..7e7400d 100644 --- a/tools/flask/policy/policy/modules/xen/xen.te +++ b/tools/flask/policy/policy/modules/xen/xen.te @@ -73,6 +73,15 @@ allow dom0_t xen_t:xen2 { pmu_ctrl get_symbol }; + +# Allow dom0 to use all XENVER_ subops +# Note that dom0 is part of domain_type so this has duplicates. +allow dom0_t xen_t:version { + xen_version xen_extraversion xen_compile_info xen_capabilities + xen_changeset xen_platform_parameters xen_get_features xen_pagesize + xen_guest_handle xen_commandline +}; + allow dom0_t xen_t:mmu memorymap; # Allow dom0 to use these domctls on itself. For domctls acting on other @@ -137,6 +146,12 @@ if (guest_writeconsole) { # pmu_ctrl is for) allow domain_type xen_t:xen2 pmu_use; +# For normal guests all except XENVER_commandline +allow domain_type xen_t:version { + xen_version xen_extraversion xen_compile_info xen_capabilities + xen_changeset xen_platform_parameters xen_get_features xen_pagesize + xen_guest_handle +}; ############################################################################### # # Domain creation diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 0618da2..2699ac0 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -13,6 +13,7 @@ #include <xen/nmi.h> #include <xen/guest_access.h> #include <xen/hypercall.h> +#include <xsm/xsm.h> #include <asm/current.h> #include <public/nmi.h> #include <public/version.h> @@ -223,12 +224,15 @@ void __init do_initcalls(void) /* * Simple hypercalls. */ - DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) { + bool_t deny = !!xsm_xen_version(XSM_OTHER, cmd); + switch ( cmd ) { case XENVER_version: + if ( deny ) + return 0; return (xen_major_version() << 16) | xen_minor_version(); case XENVER_extraversion: @@ -236,7 +240,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) xen_extraversion_t extraversion; memset(extraversion, 0, sizeof(extraversion)); - safe_strcpy(extraversion, xen_extra_version()); + safe_strcpy(extraversion, deny ? xen_deny() : xen_extra_version()); if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) ) return -EFAULT; return 0; @@ -247,10 +251,10 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) xen_compile_info_t info; memset(&info, 0, sizeof(info)); - safe_strcpy(info.compiler, xen_compiler()); - safe_strcpy(info.compile_by, xen_compile_by()); - safe_strcpy(info.compile_domain, xen_compile_domain()); - safe_strcpy(info.compile_date, xen_compile_date()); + safe_strcpy(info.compiler, deny ? xen_deny() : xen_compiler()); + safe_strcpy(info.compile_by, deny ? xen_deny() : xen_compile_by()); + safe_strcpy(info.compile_domain, deny ? xen_deny() : xen_compile_domain()); + safe_strcpy(info.compile_date, deny ? xen_deny() : xen_compile_date()); if ( copy_to_guest(arg, &info, 1) ) return -EFAULT; return 0; @@ -261,7 +265,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) xen_capabilities_info_t info; memset(info, 0, sizeof(info)); - arch_get_xen_caps(&info); + if ( !deny ) + arch_get_xen_caps(&info); if ( copy_to_guest(arg, info, ARRAY_SIZE(info)) ) return -EFAULT; @@ -274,6 +279,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) .virt_start = HYPERVISOR_VIRT_START }; + if ( deny ) + params.virt_start = 0; + if ( copy_to_guest(arg, ¶ms, 1) ) return -EFAULT; return 0; @@ -285,7 +293,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) xen_changeset_info_t chgset; memset(chgset, 0, sizeof(chgset)); - safe_strcpy(chgset, xen_changeset()); + safe_strcpy(chgset, deny ? xen_deny() : xen_changeset()); if ( copy_to_guest(arg, chgset, ARRAY_SIZE(chgset)) ) return -EFAULT; return 0; @@ -302,6 +310,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) switch ( fi.submap_idx ) { case 0: + if ( deny ) + break; fi.submap = (1U << XENFEAT_memory_op_vnode_supported); if ( VM_ASSIST(d, pae_extended_cr3) ) fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb); @@ -342,19 +352,38 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) } case XENVER_pagesize: + if ( deny ) + return 0; return (!guest_handle_is_null(arg) ? -EINVAL : PAGE_SIZE); case XENVER_guest_handle: - if ( copy_to_guest(arg, current->domain->handle, - ARRAY_SIZE(current->domain->handle)) ) + { + xen_domain_handle_t hdl; + ssize_t len; + + if ( deny ) + { + len = sizeof(hdl); + memset(&hdl, 0, len); + } else + len = ARRAY_SIZE(current->domain->handle); + + if ( copy_to_guest(arg, deny ? hdl : current->domain->handle, len ) ) return -EFAULT; return 0; - + } case XENVER_commandline: - if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) ) + { + size_t len = ARRAY_SIZE(saved_cmdline); + + if ( deny ) + len = strlen(xen_deny()) + 1; + + if ( copy_to_guest(arg, deny ? xen_deny() : saved_cmdline, len) ) return -EFAULT; return 0; } + } return -ENOSYS; } diff --git a/xen/common/version.c b/xen/common/version.c index b152e27..fc9bf42 100644 --- a/xen/common/version.c +++ b/xen/common/version.c @@ -55,3 +55,18 @@ const char *xen_banner(void) { return XEN_BANNER; } + +const char *xen_deny(void) +{ + return "<denied>"; +} + +/* + * 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/xen/version.h b/xen/include/xen/version.h index 81a3c7d..016a56c 100644 --- a/xen/include/xen/version.h +++ b/xen/include/xen/version.h @@ -12,5 +12,5 @@ unsigned int xen_minor_version(void); const char *xen_extra_version(void); const char *xen_changeset(void); const char *xen_banner(void); - +const char *xen_deny(void); #endif /* __XEN_VERSION_H__ */ diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 1d13826..94b8855 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -727,3 +727,25 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int } #endif /* CONFIG_X86 */ + +#include <public/version.h> +static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op) +{ + XSM_ASSERT_ACTION(XSM_OTHER); + switch ( op ) + { + case XENVER_version: + case XENVER_extraversion: + case XENVER_compile_info: + case XENVER_capabilities: + case XENVER_changeset: + case XENVER_platform_parameters: + case XENVER_get_features: + case XENVER_pagesize: + case XENVER_guest_handle: + /* These MUST always be accessible to any guest by default. */ + return xsm_default_action(XSM_HOOK, current->domain, NULL); + default: + return xsm_default_action(XSM_PRIV, current->domain, NULL); + } +} diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 3afed70..db440f6 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -193,6 +193,7 @@ struct xsm_operations { int (*ioport_mapping) (struct domain *d, uint32_t s, uint32_t e, uint8_t allow); int (*pmu_op) (struct domain *d, unsigned int op); #endif + int (*xen_version) (uint32_t cmd); }; #ifdef CONFIG_XSM @@ -731,6 +732,10 @@ static inline int xsm_pmu_op (xsm_default_t def, struct domain *d, unsigned int #endif /* CONFIG_X86 */ +static inline int xsm_xen_version (xsm_default_t def, uint32_t op) +{ + return xsm_ops->xen_version(op); +} #endif /* XSM_NO_WRAPPERS */ #ifdef CONFIG_MULTIBOOT diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 0f32636..9791ad4 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -162,4 +162,5 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, ioport_mapping); set_to_dummy_if_null(ops, pmu_op); #endif + set_to_dummy_if_null(ops, xen_version); } diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 4813623..d1bef43 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -26,6 +26,7 @@ #include <public/xen.h> #include <public/physdev.h> #include <public/platform.h> +#include <public/version.h> #include <public/xsm/flask_op.h> @@ -1620,6 +1621,47 @@ static int flask_pmu_op (struct domain *d, unsigned int op) } #endif /* CONFIG_X86 */ +static int flask_xen_version (uint32_t op) +{ + u32 dsid = domain_sid(current->domain); + + switch ( op ) + { + case XENVER_version: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_VERSION, NULL); + case XENVER_extraversion: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_EXTRAVERSION, NULL); + case XENVER_compile_info: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_COMPILE_INFO, NULL); + case XENVER_capabilities: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_CAPABILITIES, NULL); + case XENVER_changeset: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_CHANGESET, NULL); + case XENVER_platform_parameters: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_PLATFORM_PARAMETERS, NULL); + case XENVER_get_features: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_GET_FEATURES, NULL); + case XENVER_pagesize: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_PAGESIZE, NULL); + case XENVER_guest_handle: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_GUEST_HANDLE, NULL); + case XENVER_commandline: + return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION, + VERSION__XEN_COMMANDLINE, NULL); + default: + return -EPERM; + } +} + long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op); int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op); @@ -1758,6 +1800,7 @@ static struct xsm_operations flask_ops = { .ioport_mapping = flask_ioport_mapping, .pmu_op = flask_pmu_op, #endif + .xen_version = flask_xen_version, }; static __init void flask_init(void) diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index effb59f..628dd5c 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -495,3 +495,31 @@ class security # remove ocontext label definitions for resources del_ocontext } + +# Class version is used to describe the XENVER_ hypercall. +# Each sub-ops is described here - in the default case all of them should +# be allowed except the XENVER_commandline. +# +class version +{ +# Often called by PV kernels to force an callback. + xen_version +# Extra informations (-unstable). + xen_extraversion +# Compile information of the hypervisor. + xen_compile_info +# Such as "xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64". + xen_capabilities +# Such as the virtual address of where the hypervisor resides. + xen_platform_parameters +# Source code changeset. + xen_changeset +# The features the hypervisor supports. + xen_get_features +# Page size the hypervisor uses. + xen_pagesize +# An value that the control stack can choose. + xen_guest_handle +# Xen command line. + xen_commandline +} diff --git a/xen/xsm/flask/policy/security_classes b/xen/xsm/flask/policy/security_classes index ca191db..cde4e1a 100644 --- a/xen/xsm/flask/policy/security_classes +++ b/xen/xsm/flask/policy/security_classes @@ -18,5 +18,6 @@ class shadow class event class grant class security +class version # FLASK
All of XENVER_* have now an XSM check for their sub-ops. The subop for XENVER_commandline is now a priviliged operation. To not break guests we still return an string - but it is just '<denied>\0'. The rest: XENVER_[version|extraversion|capabilities| parameters|get_features|page_size|guest_handle|changeset| compile_info] behave as before - allowed by default for all guests if using the XSM default policy or with the dummy one. The admin can choose to change the sub-ops to be denied as they see fit. Also we add a local variable block. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> v2: Do XSM check for all the XENVER_ ops. v3: Add empty data conditions. v4: Return <denied> for priv subops. v5: Move extraversion from priv to normal. Drop the XSM check for the non-priv subops. v6: Add +1 for strlen(xen_deny()) to include NULL. Move changeset, compile_info to non-priv subops. v7: Remove the \0 on xen_deny() v8: Add new XSM domain for xenver hypercall. Add all subops to it. v9: Remove the extra line, Add Ack from Daniel v10: Rename the XSM from xen_version_op to xsm_xen_version. Prefix the types with 'xen' to distinguish it from another hypercall performing similar operation. Removed Ack from Daniel as it was so large. Add local variable block. --- tools/flask/policy/policy/modules/xen/xen.te | 15 ++++++++ xen/common/kernel.c | 53 +++++++++++++++++++++------- xen/common/version.c | 15 ++++++++ xen/include/xen/version.h | 2 +- xen/include/xsm/dummy.h | 22 ++++++++++++ xen/include/xsm/xsm.h | 5 +++ xen/xsm/dummy.c | 1 + xen/xsm/flask/hooks.c | 43 ++++++++++++++++++++++ xen/xsm/flask/policy/access_vectors | 28 +++++++++++++++ xen/xsm/flask/policy/security_classes | 1 + 10 files changed, 172 insertions(+), 13 deletions(-)