From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AH8x227XX8cuprsh6xx6ZkboTSYgTTEGKQ5dqJmhKKFU2eu9x9Ir40/3ijVM2h2vWv4egTXu1M7f ARC-Seal: i=1; a=rsa-sha256; t=1516882071; cv=none; d=google.com; s=arc-20160816; b=aFTvAnAJWn+KrZgboyvYFlfDmiqlirEQqh9AY+m3iEKokVC87PPyWxCloc/T3W+eJB Feb5puVD737bY2RdspUFEJ1GrvsQwmt6DTK1rZSOsHFTsVuG86DYGi7Si8Mb2e11eNSz 1+L5qAw5Nv6sfYb1X1DoH+n3RaXB0Gq5M7SYMe5Rpi88bi0W6Fwgw7dgxcyPq3hL4Jv7 4p727NH8bZX3I4+AI8Pz8rb7g8CrN5kELw5AFKKulk1Kd0ANd27CH7F1vAofsV92QCrq t8d5GuP0PKwQkeDIzy6Ix//pymKfFbogx0+M2m/rW98KMCa8NjBCoIQk7nh4sg2VpqII i9RQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:arc-authentication-results; bh=ap1LXA6AvlcmqpMQwpsCVNEoeo/Z3r6y8KY7rpyng30=; b=BwIYjEw6FxasqJuhiJYNZSul/OKQkceKKVWnywzCnwm1WFMCgTt9KDTyeX1b0AMVua F+Zyb0Nd6DaHHoZoxjnQroR4eZGsXgZfi/5uOg1o35/uwv0utvNgqONGnALcbPdLV4CB 0lNV7PtUW235Rm8jYVLZMqwWqs/tHfoRtH5Bvenivgzluh1tuSiHFtCn+NL7jGecR45P jcIKKfkghJHVfjB38BrYnctPrnRWuX2Ez/tZ6vqMd93PmvbU4WDYRNJ2Xq9PNCBjxN0P uZCZBVtF4GdE3mrKTu2mylmnL2JNXJIufF7GcrCaHu3LVMU1Y11PXWwOMcsPsuQ2BF61 3SMw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of bp@alien8.de designates 2a01:4f8:190:11c2::b:1457 as permitted sender) smtp.mailfrom=bp@alien8.de Authentication-Results: mx.google.com; spf=pass (google.com: domain of bp@alien8.de designates 2a01:4f8:190:11c2::b:1457 as permitted sender) smtp.mailfrom=bp@alien8.de Date: Thu, 25 Jan 2018 13:07:43 +0100 From: Borislav Petkov To: dwmw@amazon.co.uk, tim.c.chen@linux.intel.com, pjt@google.com, jikos@kernel.org, gregkh@linux-foundation.org, dave.hansen@intel.com, mingo@kernel.org, riel@redhat.com, luto@amacapital.net, torvalds@linux-foundation.org, ak@linux.intel.com, keescook@google.com, jpoimboe@redhat.com, peterz@infradead.org, tglx@linutronix.de, hpa@zytor.com, linux-kernel@vger.kernel.org Cc: linux-tip-commits@vger.kernel.org Subject: Re: [tip:x86/pti] x86/retpoline: Fill return stack buffer on vmexit Message-ID: <20180125120743.ey32gvl5mjam4r2s@pd.tnic> References: <1515755487-8524-1-git-send-email-dwmw@amazon.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1589334225298273984?= X-GMAIL-MSGID: =?utf-8?q?1590566134812566595?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Fri, Jan 12, 2018 at 03:37:49AM -0800, tip-bot for David Woodhouse wrote: > +/* > + * On VMEXIT we must ensure that no RSB predictions learned in the guest > + * can be followed in the host, by overwriting the RSB completely. Both > + * retpoline and IBRS mitigations for Spectre v2 need this; only on future > + * CPUs with IBRS_ATT *might* it be avoided. > + */ > +static inline void vmexit_fill_RSB(void) > +{ > +#ifdef CONFIG_RETPOLINE > + unsigned long loops = RSB_CLEAR_LOOPS / 2; > + > + asm volatile (ANNOTATE_NOSPEC_ALTERNATIVE > + ALTERNATIVE("jmp 910f", > + __stringify(__FILL_RETURN_BUFFER(%0, RSB_CLEAR_LOOPS, %1)), > + X86_FEATURE_RETPOLINE) > + "910:" > + : "=&r" (loops), ASM_CALL_CONSTRAINT > + : "r" (loops) : "memory" ); > +#endif > +} Btw, this thing is a real pain to backport to older kernels without breaking kABI (I know, I know, latter sucks but it is what it is) so I'm thinking we could simplify that thing regardless. As it is, 41 bytes get replaced currently: [ 0.437005] apply_alternatives: feat: 7*32+12, old: ( (ptrval), len: 43), repl: ( (ptrval), len: 43), pad: 41 [ 0.438885] (ptrval): old_insn: eb 29 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 [ 0.440001] (ptrval): rpl_insn: 48 c7 c0 10 00 00 00 e8 07 00 00 00 f3 90 0f ae e8 eb f9 e8 07 00 00 00 f3 90 0f ae e8 eb f9 48 ff c8 75 e3 48 81 c4 00 01 00 00 [ 0.444002] (ptrval): final_insn: 48 c7 c0 10 00 00 00 e8 07 00 00 00 f3 90 0f ae e8 eb f9 e8 07 00 00 00 f3 90 0f ae e8 eb f9 48 ff c8 75 e3 48 81 c4 00 01 00 00 Not that it doesn't work but the less bytes we replace, the better. So it could be made to simply call two functions. The replacing then turns into: [ 0.438154] apply_alternatives: feat: 7*32+12, old: (ffffffff81060b60 len: 5), repl: (ffffffff82434692, len: 5), pad: 0 [ 0.440002] ffffffff81060b60: old_insn: e8 ab 73 01 00 [ 0.441003] ffffffff82434692: rpl_insn: e8 89 38 c4 fe [ 0.441966] apply_alternatives: Fix CALL offset: 0x173bb, CALL 0xffffffff81077f20 [ 0.444002] ffffffff81060b60: final_insn: e8 bb 73 01 00 The "old_insn" above is: ffffffff81060b60: e8 ab 73 01 00 callq ffffffff81077f10 <__fill_rsb_nop> and final_insn is ffffffff82434692: e8 89 38 c4 fe callq ffffffff81077f20 <__fill_rsb> so both CALLs with immediates for which there's no speculation going on. I had to add a new alternative_void_call() macro for that but that's straight-forward. Also, this causes objtool to segfault so Josh and I need to look at that first. Other than that, it gets a lot simpler this way: -- diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index cf5961ca8677..a84863c1b7d3 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -210,6 +210,11 @@ static inline int alternatives_text_reserved(void *start, void *end) asm volatile (ALTERNATIVE("call %P[old]", "call %P[new]", feature) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) +/* Like alternative_io, but for replacing a direct call with another one. */ +#define alternative_void_call(oldfunc, newfunc, feature, input...) \ + asm volatile (ALTERNATIVE("call %P[old]", "call %P[new]", feature) \ + : : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) + /* * Like alternative_call, but there are two features and respective functions. * If CPU has feature2, function2 is used. diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index 4ad41087ce0e..5ba951c208af 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ -#ifndef __NOSPEC_BRANCH_H__ -#define __NOSPEC_BRANCH_H__ +#ifndef __X86_ASM_NOSPEC_BRANCH_H__ +#define __X86_ASM_NOSPEC_BRANCH_H__ #include #include @@ -206,17 +206,9 @@ extern char __indirect_thunk_end[]; static inline void vmexit_fill_RSB(void) { #ifdef CONFIG_RETPOLINE - unsigned long loops; - - asm volatile (ANNOTATE_NOSPEC_ALTERNATIVE - ALTERNATIVE("jmp 910f", - __stringify(__FILL_RETURN_BUFFER(%0, RSB_CLEAR_LOOPS, %1)), - X86_FEATURE_RETPOLINE) - "910:" - : "=r" (loops), ASM_CALL_CONSTRAINT - : : "memory" ); + alternative_void_call(__fill_rsb_nop, __fill_rsb, X86_FEATURE_RETPOLINE, ASM_NO_INPUT_CLOBBER("memory")); #endif } #endif /* __ASSEMBLY__ */ -#endif /* __NOSPEC_BRANCH_H__ */ +#endif /* __X86_ASM_NOSPEC_BRANCH_H__ */ diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index d3a67fba200a..b6a002bf893b 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -971,4 +971,9 @@ bool xen_set_default_idle(void); void stop_this_cpu(void *dummy); void df_debug(struct pt_regs *regs, long error_code); + +#ifdef CONFIG_RETPOLINE +void __fill_rsb_nop(void); +void __fill_rsb(void); +#endif #endif /* _ASM_X86_PROCESSOR_H */ diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 390b3dc3d438..16cc2e73d17d 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -281,3 +281,19 @@ ssize_t cpu_show_spectre_v2(struct device *dev, return sprintf(buf, "%s\n", spectre_v2_strings[spectre_v2_enabled]); } #endif + +#ifdef CONFIG_RETPOLINE +void __fill_rsb_nop(void) +{ + cpu_relax(); +} + +void __fill_rsb(void) +{ + unsigned long loops; + + asm volatile (__stringify(__FILL_RETURN_BUFFER(%0, RSB_CLEAR_LOOPS, %1)) + : "=r" (loops), ASM_CALL_CONSTRAINT + : : "memory" ); +} +#endif Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.