| Message ID | 20231128100352.35430-4-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/x86: add testing for self modifying code and livepatch | expand |
On Tue, 28 Nov 2023, Roger Pau Monne wrote: > Introduce a helper to perform checks related to self modifying code, and start > by creating a simple test to check that alternatives have been applied. > > Such test is hooked into the boot process and called just after alternatives > have been applied. In case of failure a message is printed, and the hypervisor > is tainted as not having passed the tests, this does require introducing a new > taint bit (printed as 'A'). > > A new sysctl is also introduced to run the tests on demand. While there are no > current users introduced here, further changes will introduce those, and it's > helpful to have the interface defined in the sysctl header from the start. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Changes since v1: > - Rework test and interface. > --- > tools/include/xenctrl.h | 2 + > tools/libs/ctrl/xc_misc.c | 14 ++++++ > xen/arch/x86/Makefile | 1 + > xen/arch/x86/include/asm/test-smc.h | 18 ++++++++ > xen/arch/x86/setup.c | 3 ++ > xen/arch/x86/sysctl.c | 7 +++ > xen/arch/x86/test-smc.c | 68 +++++++++++++++++++++++++++++ If possible, could we name this differently? I am asking because of these: https://documentation-service.arm.com/static/5f8ea482f86e16515cdbe3c6?token= https://gitlab.com/xen-project/xen/-/blob/staging/xen/arch/arm/vsmc.c?ref_type=heads > xen/common/kernel.c | 5 ++- > xen/include/public/sysctl.h | 9 ++++ > xen/include/xen/lib.h | 1 + > 10 files changed, 126 insertions(+), 2 deletions(-) > create mode 100644 xen/arch/x86/include/asm/test-smc.h > create mode 100644 xen/arch/x86/test-smc.c > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 2ef8b4e05422..0f87ffa4affd 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -2658,6 +2658,8 @@ int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, > uint32_t overlay_fdt_size, uint8_t overlay_op); > #endif > > +int xc_test_smc(xc_interface *xch, uint32_t tests, uint32_t *result); > + > /* Compat shims */ > #include "xenctrl_compat.h" > > diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c > index 5ecdfa2c7934..7f7ece589cc2 100644 > --- a/tools/libs/ctrl/xc_misc.c > +++ b/tools/libs/ctrl/xc_misc.c > @@ -1021,6 +1021,20 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32 > return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags); > } > > +int xc_test_smc(xc_interface *xch, uint32_t tests, uint32_t *result) > +{ > + struct xen_sysctl sysctl = { > + .cmd = XEN_SYSCTL_test_smc, > + .u.smc.tests = tests, > + }; > + int rc = do_sysctl(xch, &sysctl); > + > + if ( !rc ) > + *result = sysctl.u.smc.results; > + > + return rc; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > index f629157086d0..bdd2183a2fd7 100644 > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -65,6 +65,7 @@ obj-y += smpboot.o > obj-y += spec_ctrl.o > obj-y += srat.o > obj-y += string.o > +obj-y += test-smc.o > obj-y += time.o > obj-y += traps.o > obj-y += tsx.o > diff --git a/xen/arch/x86/include/asm/test-smc.h b/xen/arch/x86/include/asm/test-smc.h > new file mode 100644 > index 000000000000..18b23dbdbf2d > --- /dev/null > +++ b/xen/arch/x86/include/asm/test-smc.h > @@ -0,0 +1,18 @@ > +#ifndef _ASM_X86_TEST_SMC_H_ > +#define _ASM_X86_TEST_SMC_H_ > + > +#include <xen/types.h> > + > +int test_smc(uint32_t selection, uint32_t *results); > + > +#endif /* _ASM_X86_TEST_SMC_H_ */ > + > +/* > + * 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/setup.c b/xen/arch/x86/setup.c > index f6b8a3efd752..1f90d30204fe 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -58,6 +58,7 @@ > #include <asm/microcode.h> > #include <asm/prot-key.h> > #include <asm/pv/domain.h> > +#include <asm/test-smc.h> > > /* opt_nosmp: If true, secondary processors are ignored. */ > static bool __initdata opt_nosmp; > @@ -1952,6 +1953,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) > > alternative_branches(); > > + test_smc(XEN_SYSCTL_TEST_SMC_ALL, NULL); > + > /* > * NB: when running as a PV shim VCPUOP_up/down is wired to the shim > * physical cpu_add/remove functions, so launch the guest with only > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c > index 1d40d82c5ad2..77d091f4bd59 100644 > --- a/xen/arch/x86/sysctl.c > +++ b/xen/arch/x86/sysctl.c > @@ -25,6 +25,7 @@ > #include <asm/processor.h> > #include <asm/setup.h> > #include <asm/smp.h> > +#include <asm/test-smc.h> > #include <asm/numa.h> > #include <xen/nodemask.h> > #include <xen/cpu.h> > @@ -423,6 +424,12 @@ long arch_do_sysctl( > break; > } > > + case XEN_SYSCTL_test_smc: > + ret = test_smc(sysctl->u.smc.tests, &sysctl->u.smc.results); > + if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.smc.results) ) > + ret = -EFAULT; > + break; > + > default: > ret = -ENOSYS; > break; > diff --git a/xen/arch/x86/test-smc.c b/xen/arch/x86/test-smc.c > new file mode 100644 > index 000000000000..8916c185d60a > --- /dev/null > +++ b/xen/arch/x86/test-smc.c > @@ -0,0 +1,68 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#include <xen/errno.h> > + > +#include <asm/alternative.h> > +#include <asm/cpufeature.h> > +#include <asm/test-smc.h> > + > +static bool cf_check test_insn_replacement(void) > +{ > +#define EXPECTED_VALUE 2 > + unsigned int r = ~EXPECTED_VALUE; > + > + alternative_io("", "mov $" STR(EXPECTED_VALUE) ", %0", > + X86_FEATURE_ALWAYS, "=r"(r)); > + > + return r == EXPECTED_VALUE; > +#undef EXPECTED_VALUE > +} > + > +int test_smc(uint32_t selection, uint32_t *results) > +{ > + struct { > + unsigned int mask; > + bool (*test)(void); > + const char *name; > + } static const tests[] = { > + { XEN_SYSCTL_TEST_SMC_INSN_REPL, &test_insn_replacement, > + "alternative instruction replacement" }, > + }; > + unsigned int i; > + > + if ( selection & ~XEN_SYSCTL_TEST_SMC_ALL ) > + return -EINVAL; > + > + if ( results ) > + *results = 0; > + > + printk(XENLOG_INFO "Checking Self Modify Code\n"); > + > + for ( i = 0; i < ARRAY_SIZE(tests); i++ ) > + { > + if ( !(selection & tests[i].mask) ) > + continue; > + > + if ( tests[i].test() ) > + { > + if ( results ) > + *results |= tests[i].mask; > + continue; > + } > + > + add_taint(TAINT_ERROR_SMC); > + printk(XENLOG_ERR "%s test failed\n", tests[i].name); > + } > + > + return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/common/kernel.c b/xen/common/kernel.c > index 08dbaa2a054c..fed7ed0d587f 100644 > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -386,13 +386,14 @@ char *print_tainted(char *str) > { > if ( tainted ) > { > - snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c%c", > + snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c%c%c", > tainted & TAINT_MACHINE_INSECURE ? 'I' : ' ', > tainted & TAINT_MACHINE_CHECK ? 'M' : ' ', > tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ', > tainted & TAINT_ERROR_INJECT ? 'E' : ' ', > tainted & TAINT_HVM_FEP ? 'H' : ' ', > - tainted & TAINT_CPU_OUT_OF_SPEC ? 'S' : ' '); > + tainted & TAINT_CPU_OUT_OF_SPEC ? 'S' : ' ', > + tainted & TAINT_ERROR_SMC ? 'A' : ' '); > } > else > { > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h > index 9b19679caeb1..94287009387c 100644 > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -1201,6 +1201,13 @@ struct xen_sysctl_dt_overlay { > }; > #endif > > +struct xen_sysctl_test_smc { > + uint32_t tests; /* IN: bitmap with selected tests to execute. */ > +#define XEN_SYSCTL_TEST_SMC_INSN_REPL (1U << 0) > +#define XEN_SYSCTL_TEST_SMC_ALL (XEN_SYSCTL_TEST_SMC_INSN_REPL) > + uint32_t results; /* OUT: test result: 1 -> success, 0 -> failure. */ > +}; > + > struct xen_sysctl { > uint32_t cmd; > #define XEN_SYSCTL_readconsole 1 > @@ -1232,6 +1239,7 @@ struct xen_sysctl { > /* #define XEN_SYSCTL_set_parameter 28 */ > #define XEN_SYSCTL_get_cpu_policy 29 > #define XEN_SYSCTL_dt_overlay 30 > +#define XEN_SYSCTL_test_smc 31 > uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */ > union { > struct xen_sysctl_readconsole readconsole; > @@ -1261,6 +1269,7 @@ struct xen_sysctl { > struct xen_sysctl_livepatch_op livepatch; > #if defined(__i386__) || defined(__x86_64__) > struct xen_sysctl_cpu_policy cpu_policy; > + struct xen_sysctl_test_smc smc; > #endif > > #if defined(__arm__) || defined (__aarch64__) > diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h > index 1793be5b6b89..1bec6a01b18a 100644 > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -167,6 +167,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c); > #define TAINT_HVM_FEP (1u << 3) > #define TAINT_MACHINE_INSECURE (1u << 4) > #define TAINT_CPU_OUT_OF_SPEC (1u << 5) > +#define TAINT_ERROR_SMC (1U << 6) > extern unsigned int tainted; > #define TAINT_STRING_MAX_LEN 20 > extern char *print_tainted(char *str); > -- > 2.43.0 >
On 28.11.2023 11:03, Roger Pau Monne wrote: > --- /dev/null > +++ b/xen/arch/x86/test-smc.c > @@ -0,0 +1,68 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#include <xen/errno.h> > + > +#include <asm/alternative.h> > +#include <asm/cpufeature.h> > +#include <asm/test-smc.h> > + > +static bool cf_check test_insn_replacement(void) > +{ > +#define EXPECTED_VALUE 2 > + unsigned int r = ~EXPECTED_VALUE; The compiler is permitted to elide the initializer unless ... > + alternative_io("", "mov $" STR(EXPECTED_VALUE) ", %0", > + X86_FEATURE_ALWAYS, "=r"(r)); ... you use "+r" here. Also (nit) there's a blank missing between that string and the opening parethesis. Also what's wrong with passing EXPECTED_VALUE in as an "i" constraint input operand? > + return r == EXPECTED_VALUE; > +#undef EXPECTED_VALUE > +} > + > +int test_smc(uint32_t selection, uint32_t *results) > +{ > + struct { > + unsigned int mask; > + bool (*test)(void); > + const char *name; > + } static const tests[] = { > + { XEN_SYSCTL_TEST_SMC_INSN_REPL, &test_insn_replacement, > + "alternative instruction replacement" }, > + }; > + unsigned int i; > + > + if ( selection & ~XEN_SYSCTL_TEST_SMC_ALL ) > + return -EINVAL; > + > + if ( results ) > + *results = 0; > + > + printk(XENLOG_INFO "Checking Self Modify Code\n"); > + > + for ( i = 0; i < ARRAY_SIZE(tests); i++ ) > + { > + if ( !(selection & tests[i].mask) ) > + continue; > + > + if ( tests[i].test() ) > + { > + if ( results ) > + *results |= tests[i].mask; > + continue; > + } > + > + add_taint(TAINT_ERROR_SMC); > + printk(XENLOG_ERR "%s test failed\n", tests[i].name); > + } > + > + return 0; > +} I think I need to look at the next patch to make sense of this. > @@ -1261,6 +1269,7 @@ struct xen_sysctl { > struct xen_sysctl_livepatch_op livepatch; > #if defined(__i386__) || defined(__x86_64__) > struct xen_sysctl_cpu_policy cpu_policy; > + struct xen_sysctl_test_smc smc; Imo the field name would better be test_smc (leaving aside Stefano's comment). Jan
On Wed, Nov 29, 2023 at 06:58:38PM -0800, Stefano Stabellini wrote: > On Tue, 28 Nov 2023, Roger Pau Monne wrote: > > Introduce a helper to perform checks related to self modifying code, and start > > by creating a simple test to check that alternatives have been applied. > > > > Such test is hooked into the boot process and called just after alternatives > > have been applied. In case of failure a message is printed, and the hypervisor > > is tainted as not having passed the tests, this does require introducing a new > > taint bit (printed as 'A'). > > > > A new sysctl is also introduced to run the tests on demand. While there are no > > current users introduced here, further changes will introduce those, and it's > > helpful to have the interface defined in the sysctl header from the start. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > Changes since v1: > > - Rework test and interface. > > --- > > tools/include/xenctrl.h | 2 + > > tools/libs/ctrl/xc_misc.c | 14 ++++++ > > xen/arch/x86/Makefile | 1 + > > xen/arch/x86/include/asm/test-smc.h | 18 ++++++++ > > xen/arch/x86/setup.c | 3 ++ > > xen/arch/x86/sysctl.c | 7 +++ > > xen/arch/x86/test-smc.c | 68 +++++++++++++++++++++++++++++ > > If possible, could we name this differently? Wikipedia also suggests 'smoc' as an alternative acronym for self-modifying code, would test-smoc be OK? Thanks, Roger.
On Thu, Nov 30, 2023 at 06:02:55PM +0100, Jan Beulich wrote: > On 28.11.2023 11:03, Roger Pau Monne wrote: > > --- /dev/null > > +++ b/xen/arch/x86/test-smc.c > > @@ -0,0 +1,68 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > + > > +#include <xen/errno.h> > > + > > +#include <asm/alternative.h> > > +#include <asm/cpufeature.h> > > +#include <asm/test-smc.h> > > + > > +static bool cf_check test_insn_replacement(void) > > +{ > > +#define EXPECTED_VALUE 2 > > + unsigned int r = ~EXPECTED_VALUE; > > The compiler is permitted to elide the initializer unless ... > > > + alternative_io("", "mov $" STR(EXPECTED_VALUE) ", %0", > > + X86_FEATURE_ALWAYS, "=r"(r)); > > ... you use "+r" here. I see, '=' assumes the operand is always written to, which is not the case if alternative is not applied. > Also (nit) there's a blank missing between that > string and the opening parethesis. Also what's wrong with passing > EXPECTED_VALUE in as an "i" constraint input operand? Me not knowing enough inline assembly I think, that's what's wrong. > > @@ -1261,6 +1269,7 @@ struct xen_sysctl { > > struct xen_sysctl_livepatch_op livepatch; > > #if defined(__i386__) || defined(__x86_64__) > > struct xen_sysctl_cpu_policy cpu_policy; > > + struct xen_sysctl_test_smc smc; > > Imo the field name would better be test_smc (leaving aside Stefano's comment). Right, will see what Stefano thinks about using test_smoc. Thanks, Roger.
On Fri, 1 Dec 2023, Roger Pau Monné wrote: > > > @@ -1261,6 +1269,7 @@ struct xen_sysctl { > > > struct xen_sysctl_livepatch_op livepatch; > > > #if defined(__i386__) || defined(__x86_64__) > > > struct xen_sysctl_cpu_policy cpu_policy; > > > + struct xen_sysctl_test_smc smc; > > > > Imo the field name would better be test_smc (leaving aside Stefano's comment). > > Right, will see what Stefano thinks about using test_smoc. If you meant "test_smoc", that's totally fine. If you meant "test_smc" I think that must be a test for the virtual SMC interface that Xen exposes, right? Good we need some tests for vsmc.c ;-)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 2ef8b4e05422..0f87ffa4affd 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -2658,6 +2658,8 @@ int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, uint32_t overlay_fdt_size, uint8_t overlay_op); #endif +int xc_test_smc(xc_interface *xch, uint32_t tests, uint32_t *result); + /* Compat shims */ #include "xenctrl_compat.h" diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c index 5ecdfa2c7934..7f7ece589cc2 100644 --- a/tools/libs/ctrl/xc_misc.c +++ b/tools/libs/ctrl/xc_misc.c @@ -1021,6 +1021,20 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32 return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout, flags); } +int xc_test_smc(xc_interface *xch, uint32_t tests, uint32_t *result) +{ + struct xen_sysctl sysctl = { + .cmd = XEN_SYSCTL_test_smc, + .u.smc.tests = tests, + }; + int rc = do_sysctl(xch, &sysctl); + + if ( !rc ) + *result = sysctl.u.smc.results; + + return rc; +} + /* * Local variables: * mode: C diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index f629157086d0..bdd2183a2fd7 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -65,6 +65,7 @@ obj-y += smpboot.o obj-y += spec_ctrl.o obj-y += srat.o obj-y += string.o +obj-y += test-smc.o obj-y += time.o obj-y += traps.o obj-y += tsx.o diff --git a/xen/arch/x86/include/asm/test-smc.h b/xen/arch/x86/include/asm/test-smc.h new file mode 100644 index 000000000000..18b23dbdbf2d --- /dev/null +++ b/xen/arch/x86/include/asm/test-smc.h @@ -0,0 +1,18 @@ +#ifndef _ASM_X86_TEST_SMC_H_ +#define _ASM_X86_TEST_SMC_H_ + +#include <xen/types.h> + +int test_smc(uint32_t selection, uint32_t *results); + +#endif /* _ASM_X86_TEST_SMC_H_ */ + +/* + * 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/setup.c b/xen/arch/x86/setup.c index f6b8a3efd752..1f90d30204fe 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -58,6 +58,7 @@ #include <asm/microcode.h> #include <asm/prot-key.h> #include <asm/pv/domain.h> +#include <asm/test-smc.h> /* opt_nosmp: If true, secondary processors are ignored. */ static bool __initdata opt_nosmp; @@ -1952,6 +1953,8 @@ void __init noreturn __start_xen(unsigned long mbi_p) alternative_branches(); + test_smc(XEN_SYSCTL_TEST_SMC_ALL, NULL); + /* * NB: when running as a PV shim VCPUOP_up/down is wired to the shim * physical cpu_add/remove functions, so launch the guest with only diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c index 1d40d82c5ad2..77d091f4bd59 100644 --- a/xen/arch/x86/sysctl.c +++ b/xen/arch/x86/sysctl.c @@ -25,6 +25,7 @@ #include <asm/processor.h> #include <asm/setup.h> #include <asm/smp.h> +#include <asm/test-smc.h> #include <asm/numa.h> #include <xen/nodemask.h> #include <xen/cpu.h> @@ -423,6 +424,12 @@ long arch_do_sysctl( break; } + case XEN_SYSCTL_test_smc: + ret = test_smc(sysctl->u.smc.tests, &sysctl->u.smc.results); + if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.smc.results) ) + ret = -EFAULT; + break; + default: ret = -ENOSYS; break; diff --git a/xen/arch/x86/test-smc.c b/xen/arch/x86/test-smc.c new file mode 100644 index 000000000000..8916c185d60a --- /dev/null +++ b/xen/arch/x86/test-smc.c @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <xen/errno.h> + +#include <asm/alternative.h> +#include <asm/cpufeature.h> +#include <asm/test-smc.h> + +static bool cf_check test_insn_replacement(void) +{ +#define EXPECTED_VALUE 2 + unsigned int r = ~EXPECTED_VALUE; + + alternative_io("", "mov $" STR(EXPECTED_VALUE) ", %0", + X86_FEATURE_ALWAYS, "=r"(r)); + + return r == EXPECTED_VALUE; +#undef EXPECTED_VALUE +} + +int test_smc(uint32_t selection, uint32_t *results) +{ + struct { + unsigned int mask; + bool (*test)(void); + const char *name; + } static const tests[] = { + { XEN_SYSCTL_TEST_SMC_INSN_REPL, &test_insn_replacement, + "alternative instruction replacement" }, + }; + unsigned int i; + + if ( selection & ~XEN_SYSCTL_TEST_SMC_ALL ) + return -EINVAL; + + if ( results ) + *results = 0; + + printk(XENLOG_INFO "Checking Self Modify Code\n"); + + for ( i = 0; i < ARRAY_SIZE(tests); i++ ) + { + if ( !(selection & tests[i].mask) ) + continue; + + if ( tests[i].test() ) + { + if ( results ) + *results |= tests[i].mask; + continue; + } + + add_taint(TAINT_ERROR_SMC); + printk(XENLOG_ERR "%s test failed\n", tests[i].name); + } + + return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 08dbaa2a054c..fed7ed0d587f 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -386,13 +386,14 @@ char *print_tainted(char *str) { if ( tainted ) { - snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c%c", + snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c%c%c", tainted & TAINT_MACHINE_INSECURE ? 'I' : ' ', tainted & TAINT_MACHINE_CHECK ? 'M' : ' ', tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ', tainted & TAINT_ERROR_INJECT ? 'E' : ' ', tainted & TAINT_HVM_FEP ? 'H' : ' ', - tainted & TAINT_CPU_OUT_OF_SPEC ? 'S' : ' '); + tainted & TAINT_CPU_OUT_OF_SPEC ? 'S' : ' ', + tainted & TAINT_ERROR_SMC ? 'A' : ' '); } else { diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 9b19679caeb1..94287009387c 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -1201,6 +1201,13 @@ struct xen_sysctl_dt_overlay { }; #endif +struct xen_sysctl_test_smc { + uint32_t tests; /* IN: bitmap with selected tests to execute. */ +#define XEN_SYSCTL_TEST_SMC_INSN_REPL (1U << 0) +#define XEN_SYSCTL_TEST_SMC_ALL (XEN_SYSCTL_TEST_SMC_INSN_REPL) + uint32_t results; /* OUT: test result: 1 -> success, 0 -> failure. */ +}; + struct xen_sysctl { uint32_t cmd; #define XEN_SYSCTL_readconsole 1 @@ -1232,6 +1239,7 @@ struct xen_sysctl { /* #define XEN_SYSCTL_set_parameter 28 */ #define XEN_SYSCTL_get_cpu_policy 29 #define XEN_SYSCTL_dt_overlay 30 +#define XEN_SYSCTL_test_smc 31 uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */ union { struct xen_sysctl_readconsole readconsole; @@ -1261,6 +1269,7 @@ struct xen_sysctl { struct xen_sysctl_livepatch_op livepatch; #if defined(__i386__) || defined(__x86_64__) struct xen_sysctl_cpu_policy cpu_policy; + struct xen_sysctl_test_smc smc; #endif #if defined(__arm__) || defined (__aarch64__) diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index 1793be5b6b89..1bec6a01b18a 100644 --- a/xen/include/xen/lib.h +++ b/xen/include/xen/lib.h @@ -167,6 +167,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c); #define TAINT_HVM_FEP (1u << 3) #define TAINT_MACHINE_INSECURE (1u << 4) #define TAINT_CPU_OUT_OF_SPEC (1u << 5) +#define TAINT_ERROR_SMC (1U << 6) extern unsigned int tainted; #define TAINT_STRING_MAX_LEN 20 extern char *print_tainted(char *str);
Introduce a helper to perform checks related to self modifying code, and start by creating a simple test to check that alternatives have been applied. Such test is hooked into the boot process and called just after alternatives have been applied. In case of failure a message is printed, and the hypervisor is tainted as not having passed the tests, this does require introducing a new taint bit (printed as 'A'). A new sysctl is also introduced to run the tests on demand. While there are no current users introduced here, further changes will introduce those, and it's helpful to have the interface defined in the sysctl header from the start. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - Rework test and interface. --- tools/include/xenctrl.h | 2 + tools/libs/ctrl/xc_misc.c | 14 ++++++ xen/arch/x86/Makefile | 1 + xen/arch/x86/include/asm/test-smc.h | 18 ++++++++ xen/arch/x86/setup.c | 3 ++ xen/arch/x86/sysctl.c | 7 +++ xen/arch/x86/test-smc.c | 68 +++++++++++++++++++++++++++++ xen/common/kernel.c | 5 ++- xen/include/public/sysctl.h | 9 ++++ xen/include/xen/lib.h | 1 + 10 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 xen/arch/x86/include/asm/test-smc.h create mode 100644 xen/arch/x86/test-smc.c