This is an archive of the discontinued LLVM Phabricator instance.

Enable the SafeStack sanitizer on CloudABI by default.
ClosedPublic

Authored by ed on Mar 27 2016, 3:13 PM.

Details

Summary

Over the last month we've been testing SafeStack extensively. As far as
we know, it works perfectly fine. That why I'd like to see us having
this enabled by default for CloudABI.

This change introduces a getDefaultSanitizers() function that toolchains
can use to specify which sanitizers are enabled by default. Once all
flags are processed, only flags that had no -fno-sanitize overrides are
enabled.

Extend the thests for CloudABI to test both the default case and the
case in which we want to explicitly disable SafeStack

Diff Detail

Event Timeline

ed updated this revision to Diff 51752.Mar 27 2016, 3:13 PM
ed retitled this revision from to Enable the SafeStack sanitizer on CloudABI by default..
ed updated this object.
ed added reviewers: kcc, pcc, eugenis.
eugenis accepted this revision.Mar 28 2016, 1:13 PM
eugenis edited edge metadata.

LGTM

This revision is now accepted and ready to land.Mar 28 2016, 1:13 PM
pcc edited edge metadata.Mar 28 2016, 1:20 PM

I assume you're aware of http://clang.llvm.org/docs/SafeStack.html#known-security-limitations and the remarks at http://www.openwall.com/lists/oss-security/2016/02/17/9 ?

But I guess it's up to you whether you want to enable this by default on your platform.

ed added a comment.Mar 29 2016, 2:17 PM
In D18505#384949, @pcc wrote:

I assume you're aware of http://clang.llvm.org/docs/SafeStack.html#known-security-limitations and the remarks at http://www.openwall.com/lists/oss-security/2016/02/17/9 ?

But I guess it's up to you whether you want to enable this by default on your platform.

Thanks for sharing those links with me! My interpretation is that the concerns raised in that mailing list post are specific to ASan, UBSan, etc. Though SafeStack is mentioned, I think the author of that email probably referred to the limitations already discussed on the first page.

The question that I asked myself was whether we should have some form of stack smashing protection enabled by default, and I think the answer to that is yes. What I take away from all of this is that SafeStack and -fstack-protector offer a similar level of protection, but the former has the advantage that the performance overhead is less. Though there may well be ways to circumvent these technologies, it doesn't justify avoiding them entirely.

This revision was automatically updated to reflect the committed changes.

We are actually considering adding stack protector cookies to safestack to get the best of the two. Since we know that there can be no overflows on the safe stack, the cookies will only be added to the unsafe stack frames, reducing the overhead.

ed added a comment.Mar 29 2016, 10:20 PM

We are actually considering adding stack protector cookies to safestack to get the best of the two. Since we know that there can be no overflows on the safe stack, the cookies will only be added to the unsafe stack frames, reducing the overhead.

That sounds interesting. Be sure to keep me in the loop if there's anything you want to have tested!