Page MenuHomePhabricator

Add the -fsanitize=shadow-call-stack flag
ClosedPublic

Authored by vlad.tsyrklevich on Mar 22 2018, 12:46 PM.

Details

Summary

Add support for the -fsanitize=shadow-call-stack flag which causes clang
to add ShadowCallStack attribute to functions compiled with that flag
enabled.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc accepted this revision.Mar 22 2018, 1:48 PM

LGTM

Please add a test for the driver functionality.

This revision is now accepted and ready to land.Mar 22 2018, 1:48 PM
  • Add Driver tests
kcc added a comment.Mar 22 2018, 9:50 PM

[didn't look at the code yet, just at the docs]

Please add a docs section describing how to handle leaf functions.
If they are not handled yet, no need to change the implementation in these pathches -- ok to do it later.

docs/ShadowCallStack.rst
14 ↗(On Diff #139554)

prologue/epilogue?
(it's your native tongue, not mine, though)

20 ↗(On Diff #139554)

Provide short comparison with RFG (more instructions, less memory, same racy attack)

38 ↗(On Diff #139554)

link to wikipedia maybe?

41 ↗(On Diff #139554)

... due to return branch predictor (or some such)

47 ↗(On Diff #139554)

Say something about attacks that first try to discover the secret location of the shadow call stack.
side channels, thread spaying, whatever you have.

74 ↗(On Diff #139554)

Please add a section that shows the assembly for the following example:

int foo() {
   return bar() + 1;
}
kcc added a comment.Mar 22 2018, 9:53 PM

please also add a short comparison with Intel CET.

vlad.tsyrklevich marked 6 inline comments as done.
  • Address Kostya's documentation feedback
kcc accepted this revision.Mar 27 2018, 5:14 PM

LGTM modulo prolog vs prlogue and epilog vs epilogue

https://en.wiktionary.org/wiki/epilog says these are alternative spellings, so up to you.

docs/ShadowCallStack.rst
14 ↗(On Diff #139554)

PTAL

docs/ShadowCallStack.rst
14 ↗(On Diff #139554)

Forgot to submit this comment: It's used both ways across LLVM but I chose to go with this one just because that's how the PrologEpilogInserter pass wrote it.

  • Small test, doc updates
This revision was automatically updated to reflect the committed changes.