This is an archive of the discontinued LLVM Phabricator instance.

[x86/SLH] Teach SLH to harden against the "ret2spec" attack by implementing the proposed mitigation technique described in the original design document.
ClosedPublic

Authored by chandlerc on Aug 16 2018, 3:19 AM.

Details

Summary

The idea is to check after calls that the return address used to arrive
at that location is in fact the correct address. In the event of
a mis-predicted return which reaches a *valid* return but not the
*correct* return, this will detect the mismatch much like it would
a mispredicted conditional branch.

This is the last published attack vector that I am aware of in the
Spectre v1 space which is not mitigated by SLH+retpolines. However,
don't read *too* much into that: this is an area of ongoing research
where we expect more issues to be discovered in the future, and it also
makes no attempt to mitigate Spectre v4. Still, this is an important
completeness bar for SLH.

The change here is of course delightfully simple. It was predicated on
cutting support for post-instruction symbols into LLVM which was not at
all simple. Many thanks to Hal Finkel, Reid Kleckner, and Justin Bogner
who helped me figure out how to do a bunch of the complex changes
involved there.

Depends on revision D50833.

Diff Detail

Event Timeline

chandlerc created this revision.Aug 16 2018, 3:19 AM
chandlerc updated this revision to Diff 160988.Aug 16 2018, 3:25 AM

Include a minor update to the documentation to make it clear that this
component is now implemented, and only jumptables remain unhandled (but can be
handled with retpolines).

chandlerc updated this revision to Diff 161903.Aug 22 2018, 2:19 AM

Update fixing a tiny bug. The tests actually cover this, but the regex hides
it.

chandlerc updated this revision to Diff 162069.Aug 22 2018, 3:12 PM

Update fixing a bug where we would try to use the red-zone mechanism to extract
the return address for functions that return twice like setjmp. This, somewhat
unsurprisingly (in retrospect) doesn't work at all. Instead, force switching
back to the simpler form where we compute the return address directly ahead of
time.

craig.topper added inline comments.
llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
2210

MachineCSE is mispelled

chandlerc marked an inline comment as done.Aug 22 2018, 3:27 PM
chandlerc added inline comments.
llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
2210

Doh, thanks, fixed.

chandlerc marked an inline comment as done.

Rebase and ping.

This patch still is waiting on review....

rnk added inline comments.Aug 27 2018, 4:56 PM
llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
2184

We can give this a useful name with something like:

MCSymbol *RetSymbol = MF.getContext().createTempSymbol("slhra", true);

Speaking of which, the second parameter to that should go away or default to true. AsmPrinter has a wrapper that does the same thing.

2205–2208

Yeah, that's a real concern... Even if we don't do it today, RA will at one point definitely want to sink this kind of LEA. You could manually do the spill yourself, and maybe mark it volatile. It adds some complexity, but this no redzone / setjmp case should be infrequent and not be performance critical code.

2239–2240

:)

llvm/test/CodeGen/X86/speculative-load-hardening-call-and-ret.ll
106

--no_x86_scrub_rip will help make this more obviously correct.

chandlerc marked 3 inline comments as done.

Update addressing review feedback.

chandlerc added inline comments.Aug 28 2018, 10:33 PM
llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
2205–2208

I'd like to do this as a follow-up though, as in practice, it does not seem to happen yet. Is that OK?

llvm/test/CodeGen/X86/speculative-load-hardening-call-and-ret.ll
106

Will regenerate this (and other) test cases with that.

FWIW, happy to have a patch flipping the default.

rnk accepted this revision.Aug 29 2018, 8:33 AM

lgtm

llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
2205–2208

Sure.

This revision is now accepted and ready to land.Aug 29 2018, 8:33 AM
This revision was automatically updated to reflect the committed changes.