linux-kernel - Re: [RFC][PATCH 5/7] x86/speculation: Change __FILL_RETURN

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]

Date:   Sun, 19 Apr 2020 11:51:55 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     alexandre.chartre@...cle.com, linux-kernel@...r.kernel.org,
        jthierry@...hat.com, tglx@...utronix.de, x86@...nel.org
Subject: Re: [RFC][PATCH 5/7] x86/speculation: Change __FILL_RETURN_BUFFER to
 work with objtool

On Thu, Apr 16, 2020 at 05:07:57PM +0200, Peter Zijlstra wrote:
> From: Alexandre Chartre <alexandre.chartre@...cle.com>
> 
> Change __FILL_RETURN_BUFFER so that the stack state is deterministically
> defined for each iteration and that objtool can have an accurate view
> of the stack.
> 
> Suggested-by: Peter Zijlstra <peterz@...radead.org>
> Signed-off-by: Alexandre Chartre <alexandre.chartre@...cle.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Link: https://lkml.kernel.org/r/20200414103618.12657-8-alexandre.chartre@oracle.com
> ---
>  arch/x86/include/asm/nospec-branch.h |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -4,6 +4,7 @@
>  #define _ASM_X86_NOSPEC_BRANCH_H_
>  
>  #include <linux/static_key.h>
> +#include <linux/frame.h>
>  
>  #include <asm/alternative.h>
>  #include <asm/alternative-asm.h>
> @@ -46,12 +47,14 @@
>  #define __FILL_RETURN_BUFFER(reg, nr, sp)	\
>  	mov	$(nr/2), reg;			\
>  771:						\
> +	ANNOTATE_INTRA_FUNCTION_CALL		\
>  	call	772f;				\
>  773:	/* speculation trap */			\
>  	pause;					\
>  	lfence;					\
>  	jmp	773b;				\
>  772:						\
> +	ANNOTATE_INTRA_FUNCTION_CALL		\
>  	call	774f;				\
>  775:	/* speculation trap */			\
>  	pause;					\
> @@ -59,8 +62,8 @@
>  	jmp	775b;				\
>  774:						\
>  	dec	reg;				\
> -	jnz	771b;				\
> -	add	$(BITS_PER_LONG/8) * nr, sp;
> +	add	$(BITS_PER_LONG/8) * 2, sp;	\
> +	jnz	771b;
>  
>  #ifdef __ASSEMBLY__

Are we still planning to warn about stack changes inside an alternative?
If so then this would still fail...

In this case I think it should be safe, but I'm not sure how we can
ensure that will always be the case for other alternatives.

And do the ORC entries actually work for this?  As far as I can tell,
they would be associated with the .altinstructions section and not
.text, so it wouldn't work.

Also, does objtool not warn about the unreachable speculation traps?
I'm guessing it doesn't notice unreachable alternatives... which I guess
is probably fine.

-- 
Josh