[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 2/4] xen/x86: introduce self modifying code test
On 14.12.2023 11:17, Roger Pau Monne wrote: > --- 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-smoc.h> > > /* opt_nosmp: If true, secondary processors are ignored. */ > static bool __initdata opt_nosmp; > @@ -1951,6 +1952,8 @@ void asmlinkage __init noreturn __start_xen(unsigned > long mbi_p) > > alternative_branches(); > > + test_smoc(XEN_SYSCTL_TEST_SMOC_ALL, NULL); I realize I'm at risk of causing scope creep, but I'd still like to at least ask: As further self-tests are added, we likely don't want to alter __start_xen() every time. Should there perhaps better be a wrapper (going forward: multiple ones, depending on the time tests want invoking), together with a Kconfig control to allow suppressing all of these tests in at least release builds? > --- /dev/null > +++ b/xen/arch/x86/test/smoc.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-smoc.h> > + > +static bool cf_check test_insn_replacement(void) > +{ > +#define EXPECTED_VALUE 2 > + unsigned int r = ~EXPECTED_VALUE; > + > + alternative_io("", "mov %1, %0", X86_FEATURE_ALWAYS, > + "+r" (r), "i" (EXPECTED_VALUE)); > + > + return r == EXPECTED_VALUE; > +#undef EXPECTED_VALUE > +} > + > +int test_smoc(uint32_t selection, uint32_t *results) > +{ > + struct { > + unsigned int mask; > + bool (*test)(void); > + const char *name; > + } static const tests[] = { > + { XEN_SYSCTL_TEST_SMOC_INSN_REPL, &test_insn_replacement, > + "alternative instruction replacement" }, > + }; > + unsigned int i; > + > + if ( selection & ~XEN_SYSCTL_TEST_SMOC_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_SMOC); > + printk(XENLOG_ERR "%s test failed\n", tests[i].name); Do we really want both of these even when coming here from the sysctl? > --- 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_SMOC ? 'A' : ' '); How well is this going to scale as other selftests are added? IOW should this taint really be self-modifying-code-specific? > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -1180,6 +1180,7 @@ struct xen_sysctl_cpu_policy { > }; > typedef struct xen_sysctl_cpu_policy xen_sysctl_cpu_policy_t; > DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t); > + > #endif > > #if defined(__arm__) || defined (__aarch64__) Stray change (perhaps leftover from moving code around)? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |