|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1] Add build-id to XENVER hypercall.
On Wed, Oct 28, 2015 at 11:42:41AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Oct 09, 2015 at 09:14:00AM -0600, Jan Beulich wrote:
> > >>> On 09.10.15 at 15:25, <konrad.wilk@xxxxxxxxxx> wrote:
> > > On Fri, Oct 09, 2015 at 01:15:42PM +0100, Andrew Cooper wrote:
> > >> On 09/10/15 09:17, Jan Beulich wrote:
> > >> >>>> On 09.10.15 at 04:56, <konrad.wilk@xxxxxxxxxx> wrote:
> > >> >> However they also change the behavior of the existing hypercall
> > >> >> for XENVER_[compile_info|changeset|commandline] and make them
> > >> >> dom0 accessible. This is if XSM is built in or not (though with
> > >> >> XSM one can expose it to a guest if desired).
> > >> > Wasn't the outcome of the previous discussion that we should not
> > >> > alter default behavior for existing sub-ops?
> > >>
> > >> I raised a worry that some guests might break if they suddenly have
> > >> access to this information cut off.
> > >
> > > Let me double-confirm that the guests are OK with this being
> > > gone. I did ran tests to see if the worked, but hadn't actually tried
> > > acessing (/sys/hypervisor/xen*) the values.
> >
> > Well, this is the kind of thing you can't find out by testing _some_
> > guest(s) - you'd need to test with all possible ones, which of course
> > is not feasible. Hence we need to be very conservative when
>
> I was thinking to test:
>
> F19-64 F19-32 F17-64 F17-32 F16-32
> F16-64 F15-32 F15-64 NetBSD FreeBSD RHEL5-64 RHEL5-32 SLES11-32 SLES12-32
> OL6_X86_64_PVHVM OL6_X86_64_PV OL5_X86_64_PVHVM Win2K (with SuSE PV drivers)
> WinXP (with Windows GPL drivers) and Windows 7 (with Windows GPL), SOL12
>
> And see what happens when those are not available (poke in /sysfs or whatever
> I can)
>
> > deciding to restrict part of a so far guest-kind-indifferent ABI.
>
> Right, so only three of them are off.
>
> Perhaps an another option would be to return success and fill out the
> value with an empty string?
>
> That actually sounds nicer.
Like this:
From f5672c4b72361132798c0ec4bd124c9ddc80bd44 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Mon, 28 Sep 2015 09:00:58 -0400
Subject: [PATCH] xsm/libxl/xen_version: Add XSM for the xen_version hypercall.
All of XENVER_* have now an XSM check.
The XENVER_[compile_info|changeset|commandline] are now
guarded by an XSM check for priviliged domains.
We allow the initial domain to see these while the other
guests are not permitted. But to not make them unhappy
we will return 0 or empty strings.
The rest: XENVER_[version|extraversion|capabilities|
parameters|get_features|page_size|guest_handle] behave
as before - allowed by default for guests (thought if the
XSM checks fail then we will return empty strings as well).
Also in case XSM for all version_* is disabled we add code
in libxl so that it won't divide by zero.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
v2: Do XSM check for all the XENVER_ ops.
v3: Add empty data conditions.
---
tools/flask/policy/policy/modules/xen/xen.te | 9 ++++
tools/libxl/libxl.c | 2 +
xen/common/kernel.c | 63 ++++++++++++++++++++++------
xen/include/xsm/dummy.h | 23 ++++++++++
xen/include/xsm/xsm.h | 6 +++
xen/xsm/dummy.c | 1 +
xen/xsm/flask/hooks.c | 27 ++++++++++++
xen/xsm/flask/policy/access_vectors | 5 +++
8 files changed, 124 insertions(+), 12 deletions(-)
diff --git a/tools/flask/policy/policy/modules/xen/xen.te
b/tools/flask/policy/policy/modules/xen/xen.te
index d35ae22..40dd7f4 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -73,6 +73,12 @@ allow dom0_t xen_t:xen2 {
pmu_ctrl
get_symbol
};
+
+# Allow dom0 to use XENVER_compile_info|changeset|commandline]
+allow dom0_t xen_t:xen2 {
+ version_priv
+};
+
allow dom0_t xen_t:mmu memorymap;
# Allow dom0 to use these domctls on itself. For domctls acting on other
@@ -137,6 +143,9 @@ if (guest_writeconsole) {
# pmu_ctrl is for)
allow domain_type xen_t:xen2 pmu_use;
+# All all domains to use (unprivileged parts of) the XENVER_* hypercall
+allow domain_type xen_t:domain2 version_use;
+
###############################################################################
#
# Domain creation
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 22bbc29..3c03b6a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5288,6 +5288,8 @@ const libxl_version_info*
libxl_get_version_info(libxl_ctx *ctx)
info->virt_start = u.p_parms.virt_start;
info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
+ if (!info->pagesize) /* No divide by zero! */
+ info->pagesize = 1;
xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
info->commandline = strdup(u.xen_commandline);
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 6a3196a..c98c58c 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>
@@ -226,12 +227,15 @@ void __init do_initcalls(void)
/*
* Simple hypercalls.
*/
-
DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
+ int empty_data = xsm_version_op(XSM_HOOK, cmd);
+
switch ( cmd )
{
case XENVER_version:
+ if ( empty_data )
+ return 0;
return (xen_major_version() << 16) | xen_minor_version();
case XENVER_extraversion:
@@ -239,7 +243,8 @@ 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());
+ if ( !empty_data )
+ safe_strcpy(extraversion, xen_extra_version());
if ( copy_to_guest(arg, extraversion, ARRAY_SIZE(extraversion)) )
return -EFAULT;
return 0;
@@ -250,10 +255,13 @@ 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());
+ if ( !empty_data )
+ {
+ 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());
+ }
if ( copy_to_guest(arg, &info, 1) )
return -EFAULT;
return 0;
@@ -264,7 +272,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 ( !empty_data )
+ arch_get_xen_caps(&info);
if ( copy_to_guest(arg, info, ARRAY_SIZE(info)) )
return -EFAULT;
@@ -277,6 +286,9 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
.virt_start = HYPERVISOR_VIRT_START
};
+ if ( empty_data )
+ params.virt_start = 0;
+
if ( copy_to_guest(arg, ¶ms, 1) )
return -EFAULT;
return 0;
@@ -288,7 +300,8 @@ 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());
+ if ( !empty_data )
+ safe_strcpy(chgset, xen_changeset());
if ( copy_to_guest(arg, chgset, ARRAY_SIZE(chgset)) )
return -EFAULT;
return 0;
@@ -302,9 +315,14 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
if ( copy_from_guest(&fi, arg, 1) )
return -EFAULT;
+ if ( empty_data )
+ memset(&fi, 0, sizeof(fi));
+
switch ( fi.submap_idx )
{
case 0:
+ if ( empty_data )
+ break;
fi.submap = (1U << XENFEAT_memory_op_vnode_supported);
if ( VM_ASSIST(d, pae_extended_cr3) )
fi.submap |= (1U << XENFEAT_pae_pgdir_above_4gb);
@@ -345,19 +363,40 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
}
case XENVER_pagesize:
+ if ( empty_data )
+ 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;
+ unsigned int len;
+
+ if ( empty_data )
+ {
+ len = sizeof(hdl);
+ memset(&hdl, 0, len);
+ } else
+ len = ARRAY_SIZE(current->domain->handle);
+
+ if ( copy_to_guest(arg, empty_data ? hdl : current->domain->handle,
+ len) )
return -EFAULT;
return 0;
-
+ }
case XENVER_commandline:
- if ( copy_to_guest(arg, saved_cmdline, ARRAY_SIZE(saved_cmdline)) )
+ {
+ xen_commandline_t empty;
+
+ if ( empty_data )
+ memset(&empty, 0, ARRAY_SIZE(empty));
+
+ if ( copy_to_guest(arg, empty_data ? empty : saved_cmdline,
+ ARRAY_SIZE(saved_cmdline)) )
return -EFAULT;
return 0;
}
+ }
return -ENOSYS;
}
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 9fe372c..7fb6949 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -720,4 +720,27 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct
domain *d, unsigned int
}
}
+#include <public/version.h>
+static XSM_INLINE int xsm_version_op (XSM_DEFAULT_ARG uint32_t op)
+{
+ XSM_ASSERT_ACTION(XSM_HOOK);
+ switch ( op )
+ {
+ case XENVER_compile_info:
+ case XENVER_changeset:
+ case XENVER_commandline:
+ return xsm_default_action(XSM_PRIV, current->domain, NULL);
+ case XENVER_version:
+ case XENVER_extraversion:
+ case XENVER_capabilities:
+ case XENVER_platform_parameters:
+ case XENVER_get_features:
+ case XENVER_pagesize:
+ case XENVER_guest_handle:
+ return xsm_default_action(XSM_HOOK, current->domain, NULL);
+ default:
+ return -EPERM;
+ }
+}
+
#endif /* CONFIG_X86 */
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index ba3caed..cd3e9b8 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -191,6 +191,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 (*version_op) (uint32_t cmd);
};
#ifdef XSM_ENABLE
@@ -727,6 +728,11 @@ static inline int xsm_pmu_op (xsm_default_t def, struct
domain *d, unsigned int
return xsm_ops->pmu_op(d, op);
}
+static inline int xsm_version_op (xsm_default_t def, uint32_t op)
+{
+ return xsm_ops->version_op(op);
+}
+
#endif /* CONFIG_X86 */
#endif /* XSM_NO_WRAPPERS */
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index 72eba40..6105c07 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, version_op);
}
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 4180f3b..a08a5fe 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>
@@ -1621,6 +1622,31 @@ static int flask_pmu_op (struct domain *d, unsigned int
op)
}
#endif /* CONFIG_X86 */
+static int flask_version_op (uint32_t op)
+{
+ u32 dsid = domain_sid(current->domain);
+
+ switch ( op )
+ {
+ case XENVER_compile_info:
+ case XENVER_changeset:
+ case XENVER_commandline:
+ return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_XEN2,
+ XEN2__VERSION_PRIV, NULL);
+ case XENVER_version:
+ case XENVER_extraversion:
+ case XENVER_capabilities:
+ case XENVER_platform_parameters:
+ case XENVER_get_features:
+ case XENVER_pagesize:
+ case XENVER_guest_handle:
+ return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_DOMAIN2,
+ DOMAIN2__VERSION_USE, 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);
@@ -1759,6 +1785,7 @@ static struct xsm_operations flask_ops = {
.ioport_mapping = flask_ioport_mapping,
.pmu_op = flask_pmu_op,
#endif
+ .version_op = flask_version_op,
};
static __init void flask_init(void)
diff --git a/xen/xsm/flask/policy/access_vectors
b/xen/xsm/flask/policy/access_vectors
index effb59f..806f159 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -93,6 +93,8 @@ class xen2
pmu_ctrl
# PMU use (domains, including unprivileged ones, will be using this operation)
pmu_use
+# XENVER_[compile_info|changeset|commandline] usage.
+ version_priv
}
# Classes domain and domain2 consist of operations that a domain performs on
@@ -242,6 +244,9 @@ class domain2
mem_sharing
# XEN_DOMCTL_psr_cat_op
psr_cat_op
+# XENVER_[version|extraversion|capabilities|parameters|get_features|
+# page_size|guest_handle] usage.
+ version_use
}
# Similar to class domain, but primarily contains domctls related to HVM
domains
--
2.1.0
> >
> > Jan
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |