From cbb3dfb01d9db70f5b68a0cc3ec9e87f5276b6f4 Mon Sep 17 00:00:00 2001 From: Andrew Cooper Date: Sun, 17 Jul 2016 17:45:33 +0100 Subject: [PATCH] pv32pae: Probe for leaked SMEP/SMAP before switching to user mappings This allows XTF to panic() with a useful error message, rather than crashing with an obscure error on the hypervisor console because of Xen failing to create a bounce frame on the (now-user) stack. Update the documentation, as Xen 4.7 no longer leaks SMEP/SMAP into guests. Signed-off-by: Andrew Cooper --- arch/x86/pv/traps.c | 78 +++++++++++++++++++++++++++++++++++---------- arch/x86/setup.c | 5 ++- common/exlog.c | 1 - common/lib.c | 1 - docs/mainpage.dox | 9 +++--- include/xtf.h | 1 - include/xtf/lib.h | 1 + 7 files changed, 69 insertions(+), 27 deletions(-) diff --git a/arch/x86/pv/traps.c b/arch/x86/pv/traps.c index ed98dc6..f38e3f0 100644 --- a/arch/x86/pv/traps.c +++ b/arch/x86/pv/traps.c @@ -7,6 +7,7 @@ #include #include #include +#include /* Real entry points */ void entry_DE(void); @@ -57,6 +58,22 @@ struct xen_trap_info pv_default_trap_info[] = { 0, 0, 0, 0 }, /* Sentinel. */ }; +#ifdef __i386__ +static bool __used ex_pf_user(struct cpu_regs *regs, + const struct extable_entry *ex) +{ + if ( regs->entry_vector == X86_EXC_PF && read_cr2() == 0xfff ) + { + regs->ax = true; + regs->ip = ex->fixup; + + return true; + } + + return false; +} +#endif + void arch_init_traps(void) { /* PV equivalent of `lidt`. */ @@ -75,11 +92,6 @@ void arch_init_traps(void) write_fs(__USER_DS); write_gs(__USER_DS); - /* Unmap page at 0 to catch errors with NULL pointers. */ - rc = hypercall_update_va_mapping(NULL, 0, UVMF_INVLPG); - if ( rc ) - panic("Failed to unmap page at NULL: %d\n", rc); - #ifdef __x86_64__ /* * Set the user pagetables (only applicable to 64bit PV). @@ -101,22 +113,49 @@ void arch_init_traps(void) if ( test_wants_user_mappings ) { /* - * Walk the live pagetables and set _PAGE_USER - * * XTF uses a shared user/kernel address space, and _PAGE_USER must be * set to permit cpl3 access to the virtual addresses without taking a * pagefault. * - * !!! WARNING !!! - * - * Because PV guests and Xen share CR4, Xen's setting of - * CR4.{SMEP,SMAP} interfere with 32bit PV guests. For now, 32bit PV - * XTF tests require the hypervisor to be booted with "smep=0 smap=0". - * - * TODO - figure out how to work around this restriction while - * maintaining a shared user/kernel address space for the framework. - * - * !!! WARNING !!! + * PV guests and Xen share a virtual address space, and before Xen + * 4.7, Xen's setting of CR4.{SMEP,SMAP} leaked with 32bit PV guests. + * On hardware which supports SMEP/SMAP, older versions of Xen must be + * booted with 'smep=0 smap=0' for pv32pae tests to run. + */ + + /* + * First, probe whether Xen is leaking its SMEP/SMAP settings. + */ + intpte_t nl1e = pte_from_gfn(pfn_to_mfn(0), PF_SYM(AD, U, RW, P)); + bool leaked = false; + + /* Remap the page at 0 with _PAGE_USER. */ + rc = hypercall_update_va_mapping(NULL, nl1e, UVMF_INVLPG); + if ( rc ) + panic("Failed to remap page at NULL with _PAGE_USER: %d\n", rc); + + /* + * Write a `ret` instruction into the page at 0 (will be caught by + * leaked SMAP), then attempt to call at the `ret` instruction (will + * be caught by leaked SMEP). + */ + asm volatile ("1: movb $0xc3, (%[ptr]);" + "call *%[ptr];" + "jmp 3f;" + "2: ret;" + "3:" + _ASM_EXTABLE_HANDLER(1b, 3b, ex_pf_user) + _ASM_EXTABLE_HANDLER(0xfff, 2b, ex_pf_user) + : "+a" (leaked) + : [ptr] "r" (0xfff)); + + if ( leaked ) + panic("Xen's SMEP/SMAP settings leaked into guest context.\n" + "Must boot this Xen with 'smep=0 smap=0' to run this test.\n"); + + /* + * If we have got this far, SMEP/SMAP are not leaking into guest + * context. Proceed with remapping all mappings as _PAGE_USER. */ uint64_t *l3 = _p(start_info->pt_base); unsigned long va = 0; @@ -162,6 +201,11 @@ void arch_init_traps(void) } } #endif + + /* Unmap page at 0 to catch errors with NULL pointers. */ + rc = hypercall_update_va_mapping(NULL, 0, UVMF_INVLPG); + if ( rc ) + panic("Failed to unmap page at NULL: %d\n", rc); } void __noreturn arch_crash_hard(void) diff --git a/arch/x86/setup.c b/arch/x86/setup.c index 7f4b68f..8560001 100644 --- a/arch/x86/setup.c +++ b/arch/x86/setup.c @@ -1,5 +1,4 @@ #include -#include #include #include @@ -181,12 +180,12 @@ void arch_setup(void) collect_cpuid(IS_DEFINED(CONFIG_PV) ? pv_cpuid_count : cpuid_count); + sort_extable(); + arch_init_traps(); init_hypercalls(); - sort_extable(); - setup_pv_console(); } diff --git a/common/exlog.c b/common/exlog.c index 80efba7..f9a0735 100644 --- a/common/exlog.c +++ b/common/exlog.c @@ -1,5 +1,4 @@ #include -#include #include #include diff --git a/common/lib.c b/common/lib.c index f380f5d..9dca3e3 100644 --- a/common/lib.c +++ b/common/lib.c @@ -1,6 +1,5 @@ #include #include -#include #include void __noreturn panic(const char *fmt, ...) diff --git a/docs/mainpage.dox b/docs/mainpage.dox index c7c3d7e..2639423 100644 --- a/docs/mainpage.dox +++ b/docs/mainpage.dox @@ -54,10 +54,11 @@ To run tests on a Xen host: (see @ref errata first) @section errata Errata -- Running 32bit PV tests requires booting Xen with `"smep=0"` on IvyBridge and - newer hardware, and with `"smap=0"` on Broadwell and newer hardware, as - there is not clean separation between the paging settings of Xen and the - guest. +- Xen 4.7 and earlier running on hardware with SMEP (Intel IvyBridge/AMD + Excavator and later) or SMAP (Intel Broadwell/AMD Zen or later) leaked its + SMEP/SMAP settings into 32bit PV guests, which interferes with some tests. + To run those tests against Xen 4.6 or earlier on that hardware, Xen must be + booted with `"smep=0 smap=0"`. - For the paths of binaries, `xl` accepts either an absolute path, or certain relative paths (`/etc/xen/` or `$CWD` for `kernel=`, `$libdir/xen/boot` for diff --git a/include/xtf.h b/include/xtf.h index 725fd04..f4cb2d9 100644 --- a/include/xtf.h +++ b/include/xtf.h @@ -19,7 +19,6 @@ #include /* Optional functionality */ -#include #include #include #include diff --git a/include/xtf/lib.h b/include/xtf/lib.h index 6bece8e..36b018b 100644 --- a/include/xtf/lib.h +++ b/include/xtf/lib.h @@ -2,6 +2,7 @@ #define XTF_LIB_H #include +#include #include #if defined(__i386__) -- 2.39.5