From patchwork Tue Mar 22 16:10:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 8643961 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 12897C0553 for ; Tue, 22 Mar 2016 16:13:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 82CF320340 for ; Tue, 22 Mar 2016 16:13:44 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E69E3202B8 for ; Tue, 22 Mar 2016 16:13:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aiOtO-00065m-9u; Tue, 22 Mar 2016 16:10:50 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aiOtN-00065g-GO for xen-devel@lists.xenproject.org; Tue, 22 Mar 2016 16:10:49 +0000 Received: from [85.158.137.68] by server-3.bemta-3.messagelabs.com id 09/1C-03294-88E61F65; Tue, 22 Mar 2016 16:10:48 +0000 X-Env-Sender: konrad@char.us.oracle.com X-Msg-Ref: server-9.tower-31.messagelabs.com!1458663046!5692856!1 X-Originating-IP: [141.146.126.69] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTQxLjE0Ni4xMjYuNjkgPT4gMjc3MjE4\n X-StarScan-Received: X-StarScan-Version: 8.11; banners=-,-,- X-VirusChecked: Checked Received: (qmail 19658 invoked from network); 22 Mar 2016 16:10:47 -0000 Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by server-9.tower-31.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 22 Mar 2016 16:10:47 -0000 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u2MGAdG5017611 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 22 Mar 2016 16:10:39 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by userv0021.oracle.com (8.13.8/8.13.8) with ESMTP id u2MGAcj2023788 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Tue, 22 Mar 2016 16:10:38 GMT Received: from abhmp0011.oracle.com (abhmp0011.oracle.com [141.146.116.17]) by userv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u2MGAbEF012642; Tue, 22 Mar 2016 16:10:37 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 22 Mar 2016 09:10:37 -0700 Received: by char.us.oracle.com (Postfix, from userid 1000) id 3A0AC6A00B7; Tue, 22 Mar 2016 12:10:36 -0400 (EDT) Date: Tue, 22 Mar 2016 12:10:36 -0400 From: Konrad Rzeszutek Wilk To: Jan Beulich , dgdegra@tycho.nsa.gov Message-ID: <20160322161036.GC20062@char.us.oracle.com> References: <1458064616-23101-1-git-send-email-konrad.wilk@oracle.com> <1458064616-23101-4-git-send-email-konrad.wilk@oracle.com> <56EBFADB02000078000DE4F6@prv-mh.provo.novell.com> <20160318172610.GA9624@char.us.oracle.com> <56EFE77102000078000DEBFD@prv-mh.provo.novell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <56EFE77102000078000DEBFD@prv-mh.provo.novell.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Source-IP: userv0021.oracle.com [156.151.31.71] Cc: Wei Liu , Stefano Stabellini , andrew.cooper3@citrix.com, Ian Jackson , mpohlack@amazon.de, ross.lagerwall@citrix.com, xen-devel@lists.xenproject.org, Daniel De Graaf , sasha.levin@oracle.com Subject: Re: [Xen-devel] [PATCH v4 03/34] xsm/xen_version: Add XSM for the xen_version hypercall X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Mar 21, 2016 at 05:22:09AM -0600, Jan Beulich wrote: > >>> On 18.03.16 at 18:26, wrote: > > On Fri, Mar 18, 2016 at 05:55:55AM -0600, Jan Beulich wrote: > >> >>> On 15.03.16 at 18:56, 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. > > > > Done! (stray change). > > Considering this I'm not puzzled by ... > > > case XENVER_guest_handle: > > - if ( copy_to_guest(arg, current->domain->handle, > > - ARRAY_SIZE(current->domain->handle)) ) > > + { > > + xen_domain_handle_t hdl; > > + > > + if ( deny ) > > + memset(&hdl, 0, ARRAY_SIZE(hdl)); > > + > > + BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl)); > > + > > + if ( copy_to_guest(arg, deny ? hdl : current->domain->handle, > > + ARRAY_SIZE(hdl) ) ) > > return -EFAULT; > > return 0; > > - > > + } > > case XENVER_commandline: > > ... this. Wow. That is some sharp eyes! > > > --- a/xen/include/xsm/dummy.h > > +++ b/xen/include/xsm/dummy.h > > @@ -727,3 +727,27 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int > > } > > > > #endif /* CONFIG_X86 */ > > + > > +#include > > +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_platform_parameters: > > + case XENVER_get_features: > > + /* The sub-ops ignores the permission check and returns data. */ > > ignore ... and return ... > > With those minor things addressed I think the patch can have my ack. Thank you! Now I just need Daniel's Ack again. From 1ccf59abdd2cd9228f0159dce77fe404d98c7300 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Fri, 11 Mar 2016 21:40:43 -0500 Subject: [PATCH] xsm/xen_version: Add XSM for most of xen_version hypercall Most 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 '\0'. The XENVER_[version|platform_parameters|get_features] - will always return an value to the guest. The rest: XENVER_[extraversion|capabilities|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. And if the system admin wants to curtail access to some of them - they can do that now with a non-default XSM policy. Also we add a local variable block. Signed-off-by: Konrad Rzeszutek Wilk Acked-by: Jan Beulich Acked-by: Daniel De Graaf --- Cc: Daniel De Graaf Cc: Ian Jackson Cc: Stefano Stabellini Cc: Wei Liu v2: Do XSM check for all the XENVER_ ops. - Add empty data conditions. - Return for priv subops. - Move extraversion from priv to normal. Drop the XSM check for the non-priv subops. v3: - Add +1 for strlen(xen_deny()) to include NULL. Move changeset, compile_info to non-priv subops. - Remove the \0 on xen_deny() - Add new XSM domain for xenver hypercall. Add all subops to it. - Remove the extra line, Add Ack from Daniel v4: - 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. v5: - Make XENVER_platform_parameters,get_features,version be excluded from the XSM check per Jans' review. Add BUILD_BUG_CHECK and fix odd line removals. Remove stray changes and fix spelling. --- tools/flask/policy/policy/modules/xen/xen.te | 14 ++++++++++ xen/common/kernel.c | 42 +++++++++++++++++++++------- xen/common/version.c | 15 ++++++++++ xen/include/xen/version.h | 1 + xen/include/xsm/dummy.h | 24 ++++++++++++++++ xen/include/xsm/xsm.h | 6 ++++ xen/xsm/dummy.c | 1 + xen/xsm/flask/hooks.c | 39 ++++++++++++++++++++++++++ xen/xsm/flask/policy/access_vectors | 25 +++++++++++++++++ xen/xsm/flask/policy/security_classes | 1 + 10 files changed, 158 insertions(+), 10 deletions(-) diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te index d35ae22..18f49b5 100644 --- a/tools/flask/policy/policy/modules/xen/xen.te +++ b/tools/flask/policy/policy/modules/xen/xen.te @@ -73,6 +73,14 @@ allow dom0_t xen_t:xen2 { pmu_ctrl get_symbol }; + +# Allow dom0 to use all XENVER_ subops that have checks. +# Note that dom0 is part of domain_type so this has duplicates. +allow dom0_t xen_t:version { + xen_extraversion xen_compile_info xen_capabilities + xen_changeset 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 +145,12 @@ if (guest_writeconsole) { # pmu_ctrl is for) allow domain_type xen_t:xen2 pmu_use; +# For normal guests all possible except XENVER_commandline. +allow domain_type xen_t:version { + xen_extraversion xen_compile_info xen_capabilities + xen_changeset xen_pagesize xen_guest_handle +}; + ############################################################################### # # Domain creation diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 0618da2..a4a3c36 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -226,6 +227,8 @@ void __init do_initcalls(void) 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: @@ -236,7 +239,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 +250,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 +264,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; @@ -285,7 +289,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; @@ -342,19 +346,37 @@ 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; + + if ( deny ) + memset(&hdl, 0, ARRAY_SIZE(hdl)); + + BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl)); + + if ( copy_to_guest(arg, deny ? hdl : current->domain->handle, + ARRAY_SIZE(hdl) ) ) 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 ""; +} + +/* + * 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..2015c0b 100644 --- a/xen/include/xen/version.h +++ b/xen/include/xen/version.h @@ -12,5 +12,6 @@ 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..abbe282 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -727,3 +727,27 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int } #endif /* CONFIG_X86 */ + +#include +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_platform_parameters: + case XENVER_get_features: + /* These sub-ops ignore the permission checks and return data. */ + return 0; + case XENVER_extraversion: + case XENVER_compile_info: + case XENVER_capabilities: + case XENVER_changeset: + 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..5ecbee0 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,11 @@ 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..2069cb3 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -26,6 +26,7 @@ #include #include #include +#include #include @@ -1620,6 +1621,43 @@ 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: + case XENVER_platform_parameters: + case XENVER_get_features: + /* These sub-ops ignore the permission checks and return data. */ + return 0; + 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_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 +1796,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..badcf1c 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -495,3 +495,28 @@ class security # remove ocontext label definitions for resources del_ocontext } + +# Class version is used to describe the XENVER_ hypercall. +# Almost all sub-ops are described here - in the default case all of them should +# be allowed except the XENVER_commandline. +# +# The ones that are omitted are XENVER_version, XENVER_platform_parameters, +# and XENVER_get_features - as they MUST always be returned to a guest. +# +class 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 +# Source code changeset. + xen_changeset +# 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