[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] x86/cet: Fix shskt manipulation error with BUGFRAME_{warn,run_fn}
This was a clear oversight in the original CET work. The BUGFRAME_run_fn and BUGFRAME_warn paths update regs->rip without an equivlenet adjustment to the shadow stack, causes IRET to suffer #CP due to the mismatch. One subtle, and therefore fragile, aspect of extable_shstk_fixup() was that it required regs->rip to have its old value as a cross-check that the correct word in the shadow stack was being adjusted. Rework extable_shstk_fixup() into fixup_exception_return() which takes ownership of the update to both the regular and shadow stacks, ensuring that the regs->rip update is ordered suitably. Use the new fixup_exception_return() for BUGFRAME_run_fn and BUGFRAME_warn to ensure that the shadow stack is updated too. Fixes: 209fb9919b50 ("x86/extable: Adjust extable handling to be shadow stack compatible") Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> --- CC: Jan Beulich <JBeulich@xxxxxxxx> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> CC: Wei Liu <wl@xxxxxxx> CC: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx> Backport to 4.14 Only compile tested so far. My one CET-SS machine is in use for other purposes right now. I'm not a massive fan of the large ifdef area. The logic could be rearranged to use IS_ENABLED(CONFIG_XEN_SHSTK) by indenting most of the function, but I can't see any way to drop the goto's, and this is certainly the least-invasive diff. --- xen/arch/x86/traps.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index e60af16ddd8c..30eefbad4863 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -777,13 +777,15 @@ static void do_reserved_trap(struct cpu_user_regs *regs) trapnr, vec_name(trapnr), regs->error_code); } -static void extable_shstk_fixup(struct cpu_user_regs *regs, unsigned long fixup) +static void fixup_exception_return(struct cpu_user_regs *regs, + unsigned long fixup) { +#ifdef CONFIG_XEN_SHSTK unsigned long ssp, *ptr, *base; asm ( "rdsspq %0" : "=r" (ssp) : "0" (1) ); if ( ssp == 1 ) - return; + goto shstk_done; ptr = _p(ssp); base = _p(get_shstk_bottom(ssp)); @@ -814,7 +816,7 @@ static void extable_shstk_fixup(struct cpu_user_regs *regs, unsigned long fixup) asm ( "wrssq %[fix], %[stk]" : [stk] "=m" (ptr[0]) : [fix] "r" (fixup) ); - return; + goto shstk_done; } } @@ -824,6 +826,12 @@ static void extable_shstk_fixup(struct cpu_user_regs *regs, unsigned long fixup) * executing the interrupted context. */ BUG(); + + shstk_done: +#endif /* CONFIG_XEN_SHSTK */ + + /* Fixup the regular stack. */ + regs->rip = fixup; } static bool extable_fixup(struct cpu_user_regs *regs, bool print) @@ -842,10 +850,7 @@ static bool extable_fixup(struct cpu_user_regs *regs, bool print) vec_name(regs->entry_vector), regs->error_code, _p(regs->rip), _p(regs->rip), _p(fixup)); - if ( IS_ENABLED(CONFIG_XEN_SHSTK) ) - extable_shstk_fixup(regs, fixup); - - regs->rip = fixup; + fixup_exception_return(regs, fixup); this_cpu(last_extable_addr) = regs->rip; return true; @@ -1138,7 +1143,7 @@ void do_invalid_op(struct cpu_user_regs *regs) void (*fn)(struct cpu_user_regs *) = bug_ptr(bug); fn(regs); - regs->rip = (unsigned long)eip; + fixup_exception_return(regs, (unsigned long)eip); return; } @@ -1159,7 +1164,7 @@ void do_invalid_op(struct cpu_user_regs *regs) case BUGFRAME_warn: printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno); show_execution_state(regs); - regs->rip = (unsigned long)eip; + fixup_exception_return(regs, (unsigned long)eip); return; case BUGFRAME_bug: -- 2.11.0
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |