|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [XTF PATCH v2] Add a Live Patch privilege check test
On Fri, Nov 18, 2016 at 09:36:06AM +0000, 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@xxxxxxxxxx>
> ---
>
> In v2:
> * Fixed review issues.
> * Rebased and added a test_title string.
>
> common/lib.c | 15 +++
> docs/all-tests.dox | 2 +
> include/xen/sysctl.h | 218
> ++++++++++++++++++++++++++++++++++++
> include/xtf/hypercall.h | 6 +
> include/xtf/lib.h | 2 +
> tests/livepatch-priv-check/Makefile | 9 ++
> tests/livepatch-priv-check/main.c | 153 +++++++++++++++++++++++++
> 7 files changed, 405 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..cc847ab 100644
> --- a/common/lib.c
> +++ b/common/lib.c
> @@ -19,6 +19,21 @@ void __noreturn panic(const char *fmt, ...)
> arch_crash_hard();
> }
>
> +int probe_sysctl_interface_version(void)
/me chuckles.
> +{
> + 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..f4006fb
> --- /dev/null
> +++ b/include/xen/sysctl.h
> @@ -0,0 +1,218 @@
> +/******************************************************************************
> + * 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;
> +
> +/*
> + * 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..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..8fb490a 100644
> --- a/include/xtf/lib.h
> +++ b/include/xtf/lib.h
> @@ -65,6 +65,8 @@ static inline void exec_user_void(void (*fn)(void))
> exec_user((void *)fn);
> }
>
> +int 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..4d64d58
> --- /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 interface_version;
> +
> +static void check_ret(int rc)
Would it make it easier (to troubleshoot if somebody did enable
the XSM checks for specifc sub-obs - if that was ever done), to
extend the funtion to have 'const char *msg'
> +{
> + switch ( rc )
> + {
> + case -EPERM:
> + printk("Xen correctly denied Live Patch calls\n");
And in all of those printk and xtf_* use the '%s: ... ", msg.
> + break;
> +
> + case -ENOSYS:
> + xtf_skip("Live Patch support not detected\n");
> + break;
> +
> + default:
> + xtf_failure("Fail: Unexpected return code %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 = 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));
And here in you could just do check_ret(__func__, hypercall_sysctl?)
Thanks for making a nice test-case for this!!
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |