linux-kernel - Re: [PATCH] x86/speculation: Use AMD specific retpoline for inline asm on AMD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180918095015.GE19234@hirez.programming.kicks-ass.net>
Date:   Tue, 18 Sep 2018 11:50:15 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Zhenzhong Duan <zhenzhong.duan@...cle.com>
Cc:     linux-kernel@...r.kernel.org, mingo@...hat.com,
        konrad.wilk@...cle.com, x86@...nel.org, dwmw@...zon.co.uk,
        tglx@...utronix.de, Srinivas REDDY Eeda <srinivas.eeda@...cle.com>,
        bp@...e.de, hpa@...or.com, dhaval.giani@...cle.com
Subject: Re: [PATCH] x86/speculation: Use AMD specific retpoline for inline
 asm on AMD

On Mon, Sep 17, 2018 at 10:17:30PM -0700, Zhenzhong Duan wrote:
> Lfence is preferred than general retpoline on AMD, add this option
> in C / inline asm just as the ASM code does.
> 
> For x86_64, it still help to have minimal retpoline for kernel even
> if gcc doesn't support it, change the inline asm for x86 so that it
> could also be used by x86_64.
> Add ANNOTATE_NOSPEC_ALTERNATIVE for i386 to avoid below warning:
> "warning: objtool: .altinstr_replacement+0x10: unsupported
> intra-function call"
> "warning: objtool: If this is a retpoline, please patch it
> in with alternatives and annotate it with ANNOTATE_NOSPEC_ALTERNATIVE."

This Changelog is almost unreadable, please rewrite.

Reverse engineering the patch you add RETPOLINE_AMD support to the
inline-asm CALL_NOSPEC so that they match the asm CALL_NOSPEC.

> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@...cle.com>
> ---
>  arch/x86/include/asm/nospec-branch.h |   23 ++++++++++++++++-------
>  1 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index fd2a8c1..2d49eab 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -170,21 +170,26 @@
>   */
>  # define CALL_NOSPEC						\
>  	ANNOTATE_NOSPEC_ALTERNATIVE				\
> -	ALTERNATIVE(						\
> +	ALTERNATIVE_2(						\
>  	ANNOTATE_RETPOLINE_SAFE					\
>  	"call *%[thunk_target]\n",				\
>  	"call __x86_indirect_thunk_%V[thunk_target]\n",		\
> -	X86_FEATURE_RETPOLINE)
> +	X86_FEATURE_RETPOLINE,					\
> +	"lfence;\n"						\
> +	ANNOTATE_RETPOLINE_SAFE					\
> +	"call *%[thunk_target]\n",				\
> +	X86_FEATURE_RETPOLINE_AMD)
>  # define THUNK_TARGET(addr) [thunk_target] "r" (addr)

That's OK.

>  
> -#elif defined(CONFIG_X86_32) && defined(CONFIG_RETPOLINE)
> +#elif defined(CONFIG_RETPOLINE)

This doesn't make any sense..

>  /*
>   * For i386 we use the original ret-equivalent retpoline, because
>   * otherwise we'll run out of registers. We don't care about CET
>   * here, anyway.
>   */
>  # define CALL_NOSPEC						\
> -	ALTERNATIVE(						\
> +	ANNOTATE_NOSPEC_ALTERNATIVE				\
> +	ALTERNATIVE_2(						\
>  	ANNOTATE_RETPOLINE_SAFE					\
>  	"call *%[thunk_target]\n",				\
>  	"       jmp    904f;\n"					\
> @@ -194,12 +199,16 @@
>  	"    	lfence;\n"					\
>  	"       jmp    902b;\n"					\
>  	"       .align 16\n"					\
> -	"903:	addl   $4, %%esp;\n"				\
> -	"       pushl  %[thunk_target];\n"			\
> +	"903:	add   $4, %%" _ASM_SP ";\n"			\
> +	"       push  %[thunk_target];\n"			\

Yeah, don't do that.

>  	"       ret;\n"						\
>  	"       .align 16\n"					\
>  	"904:	call   901b;\n",				\
> -	X86_FEATURE_RETPOLINE)
> +	X86_FEATURE_RETPOLINE,					\
> +	"lfence;\n"						\
> +	ANNOTATE_RETPOLINE_SAFE					\
> +	"call *%[thunk_target]\n",				\
> +	X86_FEATURE_RETPOLINE_AMD)

And that's OK again.