This is an archive of the discontinued LLVM Phabricator instance.

[x86/SLH] Add the design document for Speculative Load Hardening, a Spectre v1 mitigation.
ClosedPublic

Authored by chandlerc on Jul 17 2018, 9:55 AM.

Details

Summary

This was initially posted w/ the patch implementing this, got some basic
review there. Also, it is generated from a the Google doc that I shared
as part of the Speculative Load Hardening RFC and which has seen pretty
widespread review at this point.

However, as the patches are landing in LLVM, I wanted to land the docs
as well. But it seemed like a bad idea to have them in the same commit
in case of reverts or other things. So the docs are split out here.

Further review and improvements to the documentation here welcome.

I've included a fresh snapshot of the google document.

As soon as this lands, I'll mark the google document as stale and point
to the LLVM URL so we have a single home for this long-term.

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc created this revision.Jul 17 2018, 9:55 AM

Adding Sidney to review.

echristo accepted this revision.Jul 17 2018, 1:59 PM

LGTM. Small organizational thoughts, but that's it.

llvm/docs/SpeculativeLoadHardening.md
980 ↗(On Diff #155908)

You have alternatives above and alternatives here. It's a little complicated figuring out which alternatives you mean where. Thoughts on coalescing in some way? I don't have anything specific.

This revision is now accepted and ready to land.Jul 17 2018, 1:59 PM

A few minor typos and suggestions.

llvm/docs/SpeculativeLoadHardening.md
126 ↗(On Diff #155908)

Possibly add a forward reference to the "Interprocedural Checking" section, as this has not be mentioned yet.

163 ↗(On Diff #155908)

Possibly add a forward reference to line 644 in the "Hardening the address of the load" section where this is explained.

528 ↗(On Diff #155908)

I get multiplying by zero to clear bits, but did you really mean to include multiplying by one?

1055 ↗(On Diff #155908)

"hels" -> "helps"

efriedma added inline comments.
llvm/docs/SpeculativeLoadHardening.md
506 ↗(On Diff #155908)

Does spilling the register to memory potentially introduce new issues? The program could use a speculative buffer overflow to clobber it, like in variant 1.1.

chandlerc marked 4 inline comments as done.Jul 18 2018, 5:14 AM

Thanks all. Most fixes applied.

I'm gonna land so we at least have a draft and we can incrementally follow up on it.

I've responded to the deeper questions below.

llvm/docs/SpeculativeLoadHardening.md
506 ↗(On Diff #155908)

Yes.

This is a particularly hard mechanism to use to bypass the mitigation though... You need to find:

  1. A store that can clobber register slots on the stack
  2. That comes after you enter speculative execution and taint the poison
  3. And after that taint gets spilled to the stack
  4. And that comes before the taint is loaded back

It isn't clear to me that this happens much if at all. I would be fine reserving a register (all hot code i have seen does not ever spill this because it is needed so often) except for the compatibility problems with reserving a register in the ABI.

Maybe we should work within the code generator to provide a stronger guarantee around minimizing / avoiding register spills.

528 ↗(On Diff #155908)

I have no idea what I was thinking....

980 ↗(On Diff #155908)

The intent was to discuss alternatives *within* the framework of SLH above, and here to discuss alternatives to the overall approach. Not super clear though as currently written. I've tweaked the heading, but yeah, I think this may need more thought...

This revision was automatically updated to reflect the committed changes.
chandlerc marked an inline comment as done.