From: Roger Pau Monne <roger.pau@citrix.com>
To: xen-devel@lists.xenproject.org
Cc: Roger Pau Monne <roger.pau@citrix.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Ross Lagerwall <ross.lagerwall@citrix.com>, Wei Liu <wl@xen.org>,
Anthony PERARD <anthony.perard@citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <george.dunlap@citrix.com>,
Jan Beulich <jbeulich@suse.com>, Julien Grall <julien@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>
Subject: [PATCH v3 3/4] x86/livepatch: introduce a basic live patch test to gitlab CI
Date: Thu, 14 Dec 2023 11:17:18 +0100 [thread overview]
Message-ID: <20231214101719.18770-4-roger.pau@citrix.com> (raw)
In-Reply-To: <20231214101719.18770-1-roger.pau@citrix.com>
Introduce a basic livepatch test using the interface to run self modifying
tests. The introduced test relies on changing a function from returning false
to returning true.
To simplify the burden of keeping a patch that can be provided to
livepatch-build-tools, introduce two new files: one containing the unpatched
test functions, and another one that contains the patched forms of such
functions. Note that only the former is linked into the Xen image, the latter
is built but the object file is not consumed afterwards. Do this to assert
that the file containing the patched functions continues to build.
Since livepatch testing will ensure that the functions are not patched previous
the applying the livepatch, allow the livepatch related tests to fail without
tainting the hypervisor.
Note the livepatch tests are not run as part of the self modifying checks
executed during boot, as they would obviously fail.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
- Clarify comment about xor vs mov instructions for return false/true
encodings.
Changes since v1:
- New interface & test.
---
tools/misc/xen-livepatch.c | 29 ++++++++++++++++++++++++++++
xen/arch/x86/include/asm/test-smoc.h | 4 ++++
xen/arch/x86/setup.c | 2 +-
xen/arch/x86/test/Makefile | 2 ++
xen/arch/x86/test/smoc-lp-alt.c | 24 +++++++++++++++++++++++
xen/arch/x86/test/smoc-lp.c | 24 +++++++++++++++++++++++
xen/arch/x86/test/smoc.c | 11 ++++++++++-
xen/include/public/sysctl.h | 6 +++++-
8 files changed, 99 insertions(+), 3 deletions(-)
create mode 100644 xen/arch/x86/test/smoc-lp-alt.c
create mode 100644 xen/arch/x86/test/smoc-lp.c
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 5bf9d9a32b65..4ebd1b4e936d 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -37,6 +37,7 @@ void show_help(void)
" replace <name> apply <name> patch and revert all others.\n"
" unload <name> unload name <name> patch.\n"
" load <file> [flags] upload and apply <file> with name as the <file> name\n"
+ " test execute self modifying code livepatch hypervisor tests\n"
" Supported flags:\n"
" --nodeps Disable inter-module buildid dependency check.\n"
" Check only against hypervisor buildid.\n",
@@ -542,6 +543,33 @@ error:
return rc;
}
+static int test_func(int argc, char *argv[])
+{
+ uint32_t results = 0;
+ int rc;
+
+ if ( argc != 0 )
+ {
+ show_help();
+ return -1;
+ }
+
+ rc = xc_test_smoc(xch, XEN_SYSCTL_TEST_SMOC_LP, &results);
+ if ( rc )
+ {
+ fprintf(stderr, "test operation failed: %s\n", strerror(errno));
+ return -1;
+ }
+ if ( (results & XEN_SYSCTL_TEST_SMOC_LP) != XEN_SYSCTL_TEST_SMOC_LP )
+ {
+ fprintf(stderr, "some tests failed: %#x (expected %#x)\n",
+ results, XEN_SYSCTL_TEST_SMOC_LP);
+ return -1;
+ }
+
+ return 0;
+}
+
/*
* These are also functions in action_options that are called in case
* none of the ones in main_options match.
@@ -554,6 +582,7 @@ struct {
{ "list", list_func },
{ "upload", upload_func },
{ "load", load_func },
+ { "test", test_func },
};
int main(int argc, char *argv[])
diff --git a/xen/arch/x86/include/asm/test-smoc.h b/xen/arch/x86/include/asm/test-smoc.h
index 2547b925d291..d369cf39687c 100644
--- a/xen/arch/x86/include/asm/test-smoc.h
+++ b/xen/arch/x86/include/asm/test-smoc.h
@@ -5,6 +5,10 @@
int test_smoc(uint32_t selection, uint32_t *results);
+#ifdef CONFIG_LIVEPATCH
+bool cf_check test_lp_insn_replacement(void);
+#endif
+
#endif /* _ASM_X86_TEST_SMC_H_ */
/*
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index e026b0ea5adc..b97faa7a3e97 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1952,7 +1952,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
alternative_branches();
- test_smoc(XEN_SYSCTL_TEST_SMOC_ALL, NULL);
+ test_smoc(XEN_SYSCTL_TEST_SMOC_ALL & ~XEN_SYSCTL_TEST_SMOC_LP, NULL);
/*
* NB: when running as a PV shim VCPUOP_up/down is wired to the shim
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
index b504b8196659..3a5a0a98e4db 100644
--- a/xen/arch/x86/test/Makefile
+++ b/xen/arch/x86/test/Makefile
@@ -1 +1,3 @@
obj-y += smoc.o
+obj-$(CONFIG_LIVEPATCH) += smoc-lp.o # for livepatch testing
+extra-$(CONFIG_LIVEPATCH) += smoc-lp-alt.o
diff --git a/xen/arch/x86/test/smoc-lp-alt.c b/xen/arch/x86/test/smoc-lp-alt.c
new file mode 100644
index 000000000000..d054e9bedc51
--- /dev/null
+++ b/xen/arch/x86/test/smoc-lp-alt.c
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/test-smoc.h>
+
+/*
+ * Interesting case because `return false` can be encoded as `xor %eax, %eax`,
+ * which is shorter than `return true` which is encoded as a `mov $1, %eax`
+ * instruction (based on code generated by GCC 13.2 at -O2), and also shorter
+ * than the replacement `jmp` instruction.
+ */
+bool cf_check test_lp_insn_replacement(void)
+{
+ return true;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/test/smoc-lp.c b/xen/arch/x86/test/smoc-lp.c
new file mode 100644
index 000000000000..e972791480b5
--- /dev/null
+++ b/xen/arch/x86/test/smoc-lp.c
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <asm/test-smoc.h>
+
+/*
+ * Interesting case because `return false` can be encoded as `xor %eax, %eax`,
+ * which is shorter than `return true` which is encoded as a `mov $1, %eax`
+ * instruction (based on code generated by GCC 13.2 at -O2), and also shorter
+ * than the replacement `jmp` instruction.
+ */
+bool cf_check test_lp_insn_replacement(void)
+{
+ return false;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/test/smoc.c b/xen/arch/x86/test/smoc.c
index e7529f937a5a..8e74d26432ee 100644
--- a/xen/arch/x86/test/smoc.c
+++ b/xen/arch/x86/test/smoc.c
@@ -27,6 +27,10 @@ int test_smoc(uint32_t selection, uint32_t *results)
} static const tests[] = {
{ XEN_SYSCTL_TEST_SMOC_INSN_REPL, &test_insn_replacement,
"alternative instruction replacement" },
+#ifdef CONFIG_LIVEPATCH
+ { XEN_SYSCTL_TEST_SMOC_LP_INSN, &test_lp_insn_replacement,
+ "livepatch instruction replacement" },
+#endif
};
unsigned int i;
@@ -50,7 +54,12 @@ int test_smoc(uint32_t selection, uint32_t *results)
continue;
}
- add_taint(TAINT_ERROR_SMOC);
+ /*
+ * livepatch related tests don't taint the hypervisor because we also
+ * want to check the failing case.
+ */
+ if ( !(tests[i].mask & XEN_SYSCTL_TEST_SMOC_LP) )
+ add_taint(TAINT_ERROR_SMOC);
printk(XENLOG_ERR "%s test failed\n", tests[i].name);
}
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 4b17f1344732..aa4068d6996b 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1205,7 +1205,11 @@ struct xen_sysctl_dt_overlay {
struct xen_sysctl_test_smoc {
uint32_t tests; /* IN: bitmap with selected tests to execute. */
#define XEN_SYSCTL_TEST_SMOC_INSN_REPL (1U << 0)
-#define XEN_SYSCTL_TEST_SMOC_ALL (XEN_SYSCTL_TEST_SMOC_INSN_REPL)
+#define XEN_SYSCTL_TEST_SMOC_LP_INSN (1U << 1)
+#define XEN_SYSCTL_TEST_SMOC_ALL (XEN_SYSCTL_TEST_SMOC_INSN_REPL | \
+ XEN_SYSCTL_TEST_SMOC_LP_INSN)
+#define XEN_SYSCTL_TEST_SMOC_LP (XEN_SYSCTL_TEST_SMOC_LP_INSN)
+
uint32_t results; /* OUT: test result: 1 -> success, 0 -> failure. */
};
--
2.43.0
next prev parent reply other threads:[~2023-12-14 10:18 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-14 10:17 [PATCH v3 0/4] xen/x86: add testing for self modifying code and livepatch Roger Pau Monne
2023-12-14 10:17 ` [PATCH v3 1/4] x86/livepatch: align functions to ensure minimal distance between entry points Roger Pau Monne
2023-12-14 11:18 ` Jan Beulich
2023-12-14 13:37 ` Roger Pau Monné
2023-12-14 13:53 ` Jan Beulich
2023-12-14 15:20 ` Roger Pau Monné
2023-12-14 15:28 ` Jan Beulich
2023-12-14 10:17 ` [PATCH v3 2/4] xen/x86: introduce self modifying code test Roger Pau Monne
2023-12-14 11:55 ` Jan Beulich
2023-12-14 13:47 ` Roger Pau Monné
2023-12-14 13:57 ` Jan Beulich
2023-12-14 15:28 ` Roger Pau Monné
2023-12-14 15:33 ` Jan Beulich
2023-12-14 10:17 ` Roger Pau Monne [this message]
2023-12-14 12:55 ` [PATCH v3 3/4] x86/livepatch: introduce a basic live patch test to gitlab CI Jan Beulich
2023-12-14 10:17 ` [PATCH v3 4/4] automation: add x86-64 livepatching test Roger Pau Monne
2023-12-14 19:10 ` Stefano Stabellini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231214101719.18770-4-roger.pau@citrix.com \
--to=roger.pau@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=anthony.perard@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=konrad.wilk@oracle.com \
--cc=ross.lagerwall@citrix.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.