* [XTF PATCH v3] Add a Live Patch privilege check test
@ 2016-11-21 9:24 Ross Lagerwall
2016-11-21 11:53 ` Andrew Cooper
0 siblings, 1 reply; 2+ messages in thread
From: Ross Lagerwall @ 2016-11-21 9:24 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Ross Lagerwall
Add a test to check that Live Patch operations cannot be called from an
unprivileged domain.
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
Changed in v3:
* Addressed review comments.
common/lib.c | 15 +++
docs/all-tests.dox | 2 +
include/xen/sysctl.h | 211 ++++++++++++++++++++++++++++++++++++
include/xen/xen.h | 9 ++
include/xtf/hypercall.h | 6 +
include/xtf/lib.h | 6 +
tests/livepatch-priv-check/Makefile | 9 ++
tests/livepatch-priv-check/main.c | 153 ++++++++++++++++++++++++++
8 files changed, 411 insertions(+)
create mode 100755 include/xen/sysctl.h
create mode 100644 tests/livepatch-priv-check/Makefile
create mode 100644 tests/livepatch-priv-check/main.c
diff --git a/common/lib.c b/common/lib.c
index 9dca3e3..0a2b311 100644
--- a/common/lib.c
+++ b/common/lib.c
@@ -19,6 +19,21 @@ void __noreturn panic(const char *fmt, ...)
arch_crash_hard();
}
+int xtf_probe_sysctl_interface_version(void)
+{
+ int i;
+ xen_sysctl_t op = {0};
+
+ for ( i = 0; i < 128; i++ )
+ {
+ op.interface_version = i;
+ if ( hypercall_sysctl(&op) != -EACCES )
+ return i;
+ }
+
+ return -1;
+}
+
/*
* Local variables:
* mode: C
diff --git a/docs/all-tests.dox b/docs/all-tests.dox
index 4f86182..2909b85 100644
--- a/docs/all-tests.dox
+++ b/docs/all-tests.dox
@@ -22,6 +22,8 @@ and functionality.
@subpage test-invlpg - `invlpg` instruction behaviour.
+@subpage test-livepatch-priv-check - Live Patch Privilege Check
+
@subpage test-pv-iopl - IOPL emulation for PV guests.
@subpage test-swint-emulation - Software interrupt emulation for HVM guests.
diff --git a/include/xen/sysctl.h b/include/xen/sysctl.h
new file mode 100755
index 0000000..a461b1c
--- /dev/null
+++ b/include/xen/sysctl.h
@@ -0,0 +1,211 @@
+/******************************************************************************
+ * sysctl.h
+ *
+ * System management operations. For use by node control stack.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2002-2006, K Fraser
+ */
+
+#ifndef __XEN_PUBLIC_SYSCTL_H__
+#define __XEN_PUBLIC_SYSCTL_H__
+
+#include "xen.h"
+#include "physdev.h"
+
+/*
+ * XEN_SYSCTL_LIVEPATCH_op
+ *
+ * Refer to the docs/unstable/misc/livepatch.markdown
+ * for the design details of this hypercall.
+ *
+ * There are four sub-ops:
+ * XEN_SYSCTL_LIVEPATCH_UPLOAD (0)
+ * XEN_SYSCTL_LIVEPATCH_GET (1)
+ * XEN_SYSCTL_LIVEPATCH_LIST (2)
+ * XEN_SYSCTL_LIVEPATCH_ACTION (3)
+ *
+ * The normal sequence of sub-ops is to:
+ * 1) XEN_SYSCTL_LIVEPATCH_UPLOAD to upload the payload. If errors STOP.
+ * 2) XEN_SYSCTL_LIVEPATCH_GET to check the `->rc`. If -XEN_EAGAIN spin.
+ * If zero go to next step.
+ * 3) XEN_SYSCTL_LIVEPATCH_ACTION with LIVEPATCH_ACTION_APPLY to apply the patch.
+ * 4) XEN_SYSCTL_LIVEPATCH_GET to check the `->rc`. If in -XEN_EAGAIN spin.
+ * If zero exit with success.
+ */
+
+#define LIVEPATCH_PAYLOAD_VERSION 1
+/*
+ * Structure describing an ELF payload. Uniquely identifies the
+ * payload. Should be human readable.
+ * Recommended length is upto XEN_LIVEPATCH_NAME_SIZE.
+ * Includes the NUL terminator.
+ */
+#define XEN_LIVEPATCH_NAME_SIZE 128
+struct xen_livepatch_name {
+ guest_handle_64_t name; /* IN: pointer to name. */
+ uint16_t size; /* IN: size of name. May be upto
+ XEN_LIVEPATCH_NAME_SIZE. */
+ uint16_t pad[3]; /* IN: MUST be zero. */
+};
+typedef struct xen_livepatch_name xen_livepatch_name_t;
+
+/*
+ * Upload a payload to the hypervisor. The payload is verified
+ * against basic checks and if there are any issues the proper return code
+ * will be returned. The payload is not applied at this time - that is
+ * controlled by XEN_SYSCTL_LIVEPATCH_ACTION.
+ *
+ * The return value is zero if the payload was succesfully uploaded.
+ * Otherwise an EXX return value is provided. Duplicate `name` are not
+ * supported.
+ *
+ * The payload at this point is verified against basic checks.
+ *
+ * The `payload` is the ELF payload as mentioned in the `Payload format`
+ * section in the Live Patch design document.
+ */
+#define XEN_SYSCTL_LIVEPATCH_UPLOAD 0
+struct xen_sysctl_livepatch_upload {
+ xen_livepatch_name_t name; /* IN, name of the patch. */
+ uint64_t size; /* IN, size of the ELF file. */
+ guest_handle_64_t payload; /* IN, the ELF file. */
+};
+typedef struct xen_sysctl_livepatch_upload xen_sysctl_livepatch_upload_t;
+
+/*
+ * Retrieve an status of an specific payload.
+ *
+ * Upon completion the `struct xen_livepatch_status` is updated.
+ *
+ * The return value is zero on success and XEN_EXX on failure. This operation
+ * is synchronous and does not require preemption.
+ */
+#define XEN_SYSCTL_LIVEPATCH_GET 1
+
+struct xen_livepatch_status {
+#define LIVEPATCH_STATE_CHECKED 1
+#define LIVEPATCH_STATE_APPLIED 2
+ uint32_t state; /* OUT: LIVEPATCH_STATE_*. */
+ int32_t rc; /* OUT: 0 if no error, otherwise -XEN_EXX. */
+};
+typedef struct xen_livepatch_status xen_livepatch_status_t;
+
+struct xen_sysctl_livepatch_get {
+ xen_livepatch_name_t name; /* IN, name of the payload. */
+ xen_livepatch_status_t status; /* IN/OUT, state of it. */
+};
+typedef struct xen_sysctl_livepatch_get xen_sysctl_livepatch_get_t;
+
+/*
+ * Retrieve an array of abbreviated status and names of payloads that are
+ * loaded in the hypervisor.
+ *
+ * If the hypercall returns an positive number, it is the number (up to `nr`)
+ * of the payloads returned, along with `nr` updated with the number of remaining
+ * payloads, `version` updated (it may be the same across hypercalls. If it
+ * varies the data is stale and further calls could fail). The `status`,
+ * `name`, and `len`' are updated at their designed index value (`idx`) with
+ * the returned value of data.
+ *
+ * If the hypercall returns E2BIG the `nr` is too big and should be
+ * lowered. The upper limit of `nr` is left to the implemention.
+ *
+ * Note that due to the asynchronous nature of hypercalls the domain might have
+ * added or removed the number of payloads making this information stale. It is
+ * the responsibility of the toolstack to use the `version` field to check
+ * between each invocation. if the version differs it should discard the stale
+ * data and start from scratch. It is OK for the toolstack to use the new
+ * `version` field.
+ */
+#define XEN_SYSCTL_LIVEPATCH_LIST 2
+struct xen_sysctl_livepatch_list {
+ uint32_t version; /* OUT: Hypervisor stamps value.
+ If varies between calls, we are
+ * getting stale data. */
+ uint32_t idx; /* IN: Index into hypervisor list. */
+ uint32_t nr; /* IN: How many status, name, and len
+ should fill out. Can be zero to get
+ amount of payloads and version.
+ OUT: How many payloads left. */
+ uint32_t pad; /* IN: Must be zero. */
+ guest_handle_64_t status; /* OUT. Must have enough
+ space allocate for nr of them. */
+ guest_handle_64_t name; /* OUT: Array of names. Each member
+ MUST XEN_LIVEPATCH_NAME_SIZE in size.
+ Must have nr of them. */
+ guest_handle_64_t len; /* OUT: Array of lengths of name's.
+ Must have nr of them. */
+};
+typedef struct xen_sysctl_livepatch_list xen_sysctl_livepatch_list_t;
+
+/*
+ * Perform an operation on the payload structure referenced by the `name` field.
+ * The operation request is asynchronous and the status should be retrieved
+ * by using either XEN_SYSCTL_LIVEPATCH_GET or XEN_SYSCTL_LIVEPATCH_LIST hypercall.
+ */
+#define XEN_SYSCTL_LIVEPATCH_ACTION 3
+struct xen_sysctl_livepatch_action {
+ xen_livepatch_name_t name; /* IN, name of the patch. */
+#define LIVEPATCH_ACTION_UNLOAD 1
+#define LIVEPATCH_ACTION_REVERT 2
+#define LIVEPATCH_ACTION_APPLY 3
+#define LIVEPATCH_ACTION_REPLACE 4
+ uint32_t cmd; /* IN: LIVEPATCH_ACTION_*. */
+ uint32_t timeout; /* IN: Zero if no timeout. */
+ /* Or upper bound of time (ms) */
+ /* for operation to take. */
+};
+typedef struct xen_sysctl_livepatch_action xen_sysctl_livepatch_action_t;
+
+struct xen_sysctl_livepatch_op {
+ uint32_t cmd; /* IN: XEN_SYSCTL_LIVEPATCH_*. */
+ uint32_t pad; /* IN: Always zero. */
+ union {
+ xen_sysctl_livepatch_upload_t upload;
+ xen_sysctl_livepatch_list_t list;
+ xen_sysctl_livepatch_get_t get;
+ xen_sysctl_livepatch_action_t action;
+ } u;
+};
+typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
+
+struct xen_sysctl {
+ uint32_t cmd;
+#define XEN_SYSCTL_livepatch_op 27
+ uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
+ union {
+ struct xen_sysctl_livepatch_op livepatch;
+ uint8_t pad[128];
+ } u;
+};
+typedef struct xen_sysctl xen_sysctl_t;
+
+#endif /* __XEN_PUBLIC_SYSCTL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 3c3876d..067bb59 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -255,6 +255,15 @@ enum XEN_UVMF {
};
#endif
+#ifndef __ASSEMBLY__
+typedef struct {
+ union {
+ void *p;
+ uint64_t __aligned(8) q;
+ };
+} guest_handle_64_t;
+#endif
+
#endif /* XEN_PUBLIC_XEN_H */
/*
diff --git a/include/xtf/hypercall.h b/include/xtf/hypercall.h
index a37ebc2..ab4986f 100644
--- a/include/xtf/hypercall.h
+++ b/include/xtf/hypercall.h
@@ -34,6 +34,7 @@ extern uint8_t hypercall_page[PAGE_SIZE];
#include <xen/physdev.h>
#include <xen/memory.h>
#include <xen/version.h>
+#include <xen/sysctl.h>
#include <xen/hvm/hvm_op.h>
#include <xen/hvm/params.h>
@@ -114,6 +115,11 @@ static inline long hypercall_hvm_op(unsigned int cmd, void *arg)
return HYPERCALL2(long, __HYPERVISOR_hvm_op, cmd, arg);
}
+static inline long hypercall_sysctl(xen_sysctl_t *arg)
+{
+ return HYPERCALL1(long, __HYPERVISOR_sysctl, arg);
+}
+
/*
* Higher level hypercall helpers
*/
diff --git a/include/xtf/lib.h b/include/xtf/lib.h
index b0ae39d..65abdab 100644
--- a/include/xtf/lib.h
+++ b/include/xtf/lib.h
@@ -65,6 +65,12 @@ static inline void exec_user_void(void (*fn)(void))
exec_user((void *)fn);
}
+/**
+ * Probe for the SYSCTL_INTERFACE_VERSION in use by the hypervisor
+ * @returns version, or -1 on failure.
+ */
+int xtf_probe_sysctl_interface_version(void);
+
#endif /* XTF_LIB_H */
/*
diff --git a/tests/livepatch-priv-check/Makefile b/tests/livepatch-priv-check/Makefile
new file mode 100644
index 0000000..e27b9da
--- /dev/null
+++ b/tests/livepatch-priv-check/Makefile
@@ -0,0 +1,9 @@
+include $(ROOT)/build/common.mk
+
+NAME := livepatch-priv-check
+CATEGORY := functional
+TEST-ENVS := $(ALL_ENVIRONMENTS)
+
+obj-perenv += main.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/livepatch-priv-check/main.c b/tests/livepatch-priv-check/main.c
new file mode 100644
index 0000000..e8fb10b
--- /dev/null
+++ b/tests/livepatch-priv-check/main.c
@@ -0,0 +1,153 @@
+/**
+ * @file tests/livepatch-priv-check/main.c
+ * @ref test-livepatch-priv-check
+ *
+ * @page test-livepatch-priv-check Live Patch Privilege Check
+ *
+ * Checks that Xen prevents using all live patching operations and
+ * sub-operations from an unprivileged guest.
+ *
+ * @see tests/livepatch-check-priv/main.c
+ */
+#include <xtf.h>
+
+const char test_title[] = "Live Patch Privilege Check";
+
+static int sysctl_interface_version;
+
+static void check_ret(const char *test, int rc)
+{
+ switch ( rc )
+ {
+ case -EPERM:
+ printk("%s: Xen correctly denied Live Patch calls\n", test);
+ break;
+
+ case -ENOSYS:
+ xtf_skip("%s: Live Patch support not detected\n", test);
+ break;
+
+ default:
+ xtf_failure("Fail: %s: Unexpected return code %d\n", test, rc);
+ break;
+ }
+}
+
+#define TEST_NAME "foo"
+
+static void test_upload(void)
+{
+ static uint8_t payload[PAGE_SIZE];
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = sysctl_interface_version,
+ .u.livepatch = {
+ .cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD,
+ .u.upload = {
+ .name = {
+ .name.p = TEST_NAME,
+ .size = sizeof(TEST_NAME),
+ },
+ .size = PAGE_SIZE,
+ .payload.p = payload,
+ },
+ },
+ };
+
+ check_ret(__func__, hypercall_sysctl(&op));
+}
+
+#define NR_PAYLOADS 8
+
+static void test_list(void)
+{
+ char names[NR_PAYLOADS * XEN_LIVEPATCH_NAME_SIZE];
+ uint32_t lengths[NR_PAYLOADS];
+
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = sysctl_interface_version,
+ .u.livepatch = {
+ .cmd = XEN_SYSCTL_LIVEPATCH_LIST,
+ .u.list = {
+ .idx = 0,
+ .nr = NR_PAYLOADS,
+ .name.p = names,
+ .len.p = lengths,
+ },
+ },
+ };
+
+ check_ret(__func__, hypercall_sysctl(&op));
+}
+
+static void test_get(void)
+{
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = sysctl_interface_version,
+ .u.livepatch = {
+ .cmd = XEN_SYSCTL_LIVEPATCH_GET,
+ .u.get = {
+ .name = {
+ .name.p = TEST_NAME,
+ .size = sizeof(TEST_NAME),
+ },
+ },
+ },
+ };
+
+ check_ret(__func__, hypercall_sysctl(&op));
+}
+
+static void test_action(uint32_t action)
+{
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = sysctl_interface_version,
+ .u.livepatch = {
+ .cmd = XEN_SYSCTL_LIVEPATCH_ACTION,
+ .u.action = {
+ .name = {
+ .name.p = TEST_NAME,
+ .size = sizeof(TEST_NAME),
+ },
+ .cmd = action,
+ .timeout = 0,
+ },
+ },
+ };
+
+ check_ret(__func__, hypercall_sysctl(&op));
+}
+
+void test_main(void)
+{
+ sysctl_interface_version = xtf_probe_sysctl_interface_version();
+ if ( sysctl_interface_version < 0 )
+ return xtf_error("Failed to find sysctl interface version\n");
+
+ test_upload();
+ test_list();
+ test_get();
+ test_action(LIVEPATCH_ACTION_UNLOAD);
+ test_action(LIVEPATCH_ACTION_REVERT);
+ test_action(LIVEPATCH_ACTION_APPLY);
+ test_action(LIVEPATCH_ACTION_REPLACE);
+
+ xtf_success(NULL);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [XTF PATCH v3] Add a Live Patch privilege check test
2016-11-21 9:24 [XTF PATCH v3] Add a Live Patch privilege check test Ross Lagerwall
@ 2016-11-21 11:53 ` Andrew Cooper
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2016-11-21 11:53 UTC (permalink / raw)
To: Ross Lagerwall, xen-devel
On 21/11/16 09:24, Ross Lagerwall wrote:
> Add a test to check that Live Patch operations cannot be called from an
> unprivileged domain.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> and applied.
I made two very small adjustments.
> diff --git a/common/lib.c b/common/lib.c
> index 9dca3e3..0a2b311 100644
> --- a/common/lib.c
> +++ b/common/lib.c
> @@ -19,6 +19,21 @@ void __noreturn panic(const char *fmt, ...)
> arch_crash_hard();
> }
>
> +int xtf_probe_sysctl_interface_version(void)
> +{
> + int i;
> + xen_sysctl_t op = {0};
This breaks the build on Clang. Using { .cmd = 0 } instead is fine.
> +
> + for ( i = 0; i < 128; i++ )
> + {
> + op.interface_version = i;
> + if ( hypercall_sysctl(&op) != -EACCES )
> + return i;
> + }
> +
> + return -1;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/tests/livepatch-priv-check/Makefile b/tests/livepatch-priv-check/Makefile
> new file mode 100644
> index 0000000..e27b9da
> --- /dev/null
> +++ b/tests/livepatch-priv-check/Makefile
> @@ -0,0 +1,9 @@
> +include $(ROOT)/build/common.mk
> +
> +NAME := livepatch-priv-check
> +CATEGORY := functional
> +TEST-ENVS := $(ALL_ENVIRONMENTS)
I have dropped this down to pv32pae pv64 hvm32 hvm64
The hvm32pse and hvm32pae environments are an identical ABI to hvm32, so
there is no point testing them all. The multiple paging options for
32bit HVM guests is only useful for testing pagetable related code.
~Andrew
> +
> +obj-perenv += main.o
> +
> +include $(ROOT)/build/gen.mk
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-11-21 11:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-21 9:24 [XTF PATCH v3] Add a Live Patch privilege check test Ross Lagerwall
2016-11-21 11:53 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).