Re: [PATCH v3 01/13] x86/retpoline: Add initial retpoline support

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2200 bytes --]

On Sun, 2018-01-07 at 12:46 +0100, Borislav Petkov wrote:
> 
> > 
> > The other fun one for alternatives is in entry_64.S, where we really
> > need the return address of the call instruction to be *precisely* the 
> > .Lentry_SYSCALL_64_after_fastpath_call label, so we have to eschew the
> > normal NOSPEC_CALL there:
> 
> So CALL, as the doc says, pushes the offset of the *next* insn onto the
> stack and branches to the target address.
> 
> So I'm thinking, as long as the next insn doesn't move and gcc doesn't
> pad anything, you're fine.
> 
> However, I suspect that I'm missing something else here and I guess I'll
> have more clue if I look at the whole thing. So can you point me to your
> current branch so that I can take a look at the code?

http://git.infradead.org/users/dwmw2/linux-retpoline.git

In particular, this call site in entry_64.S:
http://git.infradead.org/users/dwmw2/linux-retpoline.git/blob/0f5c54a36e:/arch/x86/entry/entry_64.S#l270

It's still just unconditionally calling the out-of-line thunk and not
using ALTERNATIVE in the CONFIG_RETPOLINE case. I can't just use the
NOSPEC_CALL macro from
http://git.infradead.org/users/dwmw2/linux-retpoline.git/blob/0f5c54a36e:/arch/x86/include/asm/nospec-branch.h#l46
because of that requirement that the return address (on the stack) for
the CALL instruction must be precisely at the end, in all cases.

Each of the three alternatives *does* end with the CALL, it's just that
for the two which are shorter than the full retpoline one, they'll get
padded with NOPs at the end, so the return address on the stack *won't*
be what's expected.

Explicitly padding the alternatives with leading NOPs so that they are
all precisely the same length would work, and if the alternatives
mechanism were to pad the shorter ones with leading NOPs instead of
trailing NOPs that would *also* work (but be fairly difficult
especially to do that for oldinstr).

I'm not sure I *see* a simple answer, and it isn't really that bad to
just do what GCC is doing and unconditionally call the out-of-line
thunk. So feel free to just throw your hands up in horror and say "no,
we can't cope with that" :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5213 bytes --]