Re: [RFC][PATCH 5/7] x86/speculation: Change __FILL_RETURN_BUFFER to work with objtool

From: Josh Poimboeuf
Date: Sun Apr 19 2020 - 12:52:10 EST


On Thu, Apr 16, 2020 at 05:07:57PM +0200, Peter Zijlstra wrote:
> From: Alexandre Chartre <alexandre.chartre@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Alexandre Chartre <alexandre.chartre@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Link: https://lkml.kernel.org/r/20200414103618.12657-8-alexandre.chartre@xxxxxxxxxx
> ---
> 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