* [XTF PATCH] Add a Live Patch privilege check test
@ 2016-11-17 15:40 Ross Lagerwall
2016-11-17 16:05 ` Andrew Cooper
0 siblings, 1 reply; 2+ messages in thread
From: Ross Lagerwall @ 2016-11-17 15:40 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>
---
docs/all-tests.dox | 2 +
include/xen/sysctl.h | 220 ++++++++++++++++++++++++++++++++++++
include/xtf/hypercall.h | 6 +
tests/livepatch-priv-check/Makefile | 9 ++
tests/livepatch-priv-check/main.c | 144 +++++++++++++++++++++++
5 files changed, 381 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/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..26fc876
--- /dev/null
+++ b/include/xen/sysctl.h
@@ -0,0 +1,220 @@
+/******************************************************************************
+ * 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"
+
+typedef struct {
+ union {
+ void *p;
+ uint64_t __attribute__((aligned(8))) q;
+ };
+} guest_handle_64;
+
+#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000E
+
+/*
+ * 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 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 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 status; /* OUT. Must have enough
+ space allocate for nr of them. */
+ guest_handle_64 name; /* OUT: Array of names. Each member
+ MUST XEN_LIVEPATCH_NAME_SIZE in size.
+ Must have nr of them. */
+ guest_handle_64 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/xtf/hypercall.h b/include/xtf/hypercall.h
index a37ebc2..75c0e9d 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(void *arg)
+{
+ return HYPERCALL1(long, __HYPERVISOR_sysctl, arg);
+}
+
/*
* Higher level hypercall helpers
*/
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..357e4da
--- /dev/null
+++ b/tests/livepatch-priv-check/main.c
@@ -0,0 +1,144 @@
+/**
+ * @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>
+
+static void check_ret(int rc)
+{
+ switch ( rc )
+ {
+ case -EPERM:
+ xtf_success("Xen correctly denied Live Patch calls\n");
+ break;
+ case -ENOSYS:
+ xtf_skip("Live Patch support not detected\n");
+ break;
+ case 0:
+ xtf_failure("Live Patch call succeeded\n");
+ break;
+ default:
+ xtf_failure("Unexpected error %d\n", rc);
+ break;
+ }
+}
+
+#define TEST_NAME "foo"
+
+static void test_upload(void)
+{
+ uint8_t payload[PAGE_SIZE];
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = XEN_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(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 = XEN_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(hypercall_sysctl(&op));
+}
+
+static void test_get(void)
+{
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = XEN_SYSCTL_INTERFACE_VERSION,
+ .u.livepatch = {
+ .cmd = XEN_SYSCTL_LIVEPATCH_GET,
+ .u.get = {
+ .name = {
+ .name.p = TEST_NAME,
+ .size = sizeof(TEST_NAME),
+ },
+ },
+ },
+ };
+
+ check_ret(hypercall_sysctl(&op));
+}
+
+static void test_action(uint32_t action)
+{
+ xen_sysctl_t op =
+ {
+ .cmd = XEN_SYSCTL_livepatch_op,
+ .interface_version = XEN_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(hypercall_sysctl(&op));
+}
+
+void test_main(void)
+{
+ 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);
+}
+
+/*
+ * 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] Add a Live Patch privilege check test
2016-11-17 15:40 [XTF PATCH] Add a Live Patch privilege check test Ross Lagerwall
@ 2016-11-17 16:05 ` Andrew Cooper
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2016-11-17 16:05 UTC (permalink / raw)
To: Ross Lagerwall, xen-devel
On 17/11/16 15:40, Ross Lagerwall wrote:
> +typedef struct {
> + union {
> + void *p;
> + uint64_t __attribute__((aligned(8))) q;
> + };
> +} guest_handle_64;
> +
> +#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000E
/sigh
This is going to require some rethinking. We cannot have the unstable
ABI versions baked into the header files, as the tests wont function on
older versions of Xen.
The sysctl ABI is actually fixed. The struct is always 136 bytes long,
and has two uint32_t's at the top. XTF needs a library function to
probe which interface version is accepted by Xen.
The livepatch layout is also fixed, so the sysctl version isn't actually
relevant.
> diff --git a/include/xtf/hypercall.h b/include/xtf/hypercall.h
> index a37ebc2..75c0e9d 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(void *arg)
xen_sysctl_t * please.
> +{
> + return HYPERCALL1(long, __HYPERVISOR_sysctl, arg);
> +}
> +
> /*
> * Higher level hypercall helpers
> */
> 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..357e4da
> --- /dev/null
> +++ b/tests/livepatch-priv-check/main.c
> @@ -0,0 +1,144 @@
> +/**
> + * @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>
> +
> +static void check_ret(int rc)
> +{
> + switch ( rc )
> + {
> + case -EPERM:
> + xtf_success("Xen correctly denied Live Patch calls\n");
> + break;
Newlines after breaks please.
Also, a quirk of multiple sub-tests like this is that you shouldn't call
xtf_success multiple times. It can end up incorrectly declaring success
if a logical bug caused you to skip part of the test logic.
Use printk here, and have a single xtf_success() as the final action in
test_main().
> + case -ENOSYS:
> + xtf_skip("Live Patch support not detected\n");
> + break;
> + case 0:
> + xtf_failure("Live Patch call succeeded\n");
> + break;
I find it is helpful to manually prefix the strings with "Fail: ". It
helps visually distinguish the logging if there is a mix of steps with
different results.
> + default:
> + xtf_failure("Unexpected error %d\n", rc);
> + break;
This should be xtf_error().
~Andrew
> + }
> +}
> +
>
_______________________________________________
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-17 16:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-17 15:40 [XTF PATCH] Add a Live Patch privilege check test Ross Lagerwall
2016-11-17 16:05 ` 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).