Page MenuHomePhabricator

[sanitizers] Initial implementation for -fsanitize=init-locals
AbandonedPublic

Authored by glider on Nov 13 2018, 7:16 AM.

Details

Summary

This patch adds a new feature, -fsanitize=init-locals, which generates zero initializers for uninitialized local variables.

There's been discussions in the security community about the impact of zero-initializing all locals to prevent information leaks. The new feature shall help evaluating the pros and cons of such an approach.

Credits for the code go to Daniel Micay (original patch is at https://github.com/AndroidHardeningArchive/platform_external_clang/commit/776a0955ef6686d23a82d2e6a3cbd4a6a882c31c)

Diff Detail

Event Timeline

glider created this revision.Nov 13 2018, 7:16 AM
lebedev.ri added inline comments.
include/clang/Basic/Sanitizers.def
163

Unless i'm mistaken, I suspect you may see some surprising behavior here, unfortunately.
Bug 39425 - SanitizerOrdinal is out of bits

kcc added a comment.Nov 13 2018, 12:17 PM

This new flag inhibits the warnings from -Wuninitialized, right?
While this is fine for experimenting (and I want to have this in ToT to enable wide experimentation)
we should clearly state (in the comments) that the final intent is to make the feature work together with -Wuninitialized.

Another thing that we will need (and not necessary in the first change) is to have fine grained controls over what we zero-initialize.
We may want to make separate decisions for pointer/non-pointer scalars, PODs, arrays of {scalars, pointers, PODs}, etc.

In D54473#1297460, @kcc wrote:

This new flag inhibits the warnings from -Wuninitialized, right?
While this is fine for experimenting (and I want to have this in ToT to enable wide experimentation)
we should clearly state (in the comments) that the final intent is to make the feature work together with -Wuninitialized.

No, as far as I can see, the warnings from -Wuninitialized are still present (see the test).

Another thing that we will need (and not necessary in the first change) is to have fine grained controls over what we zero-initialize.
We may want to make separate decisions for pointer/non-pointer scalars, PODs, arrays of {scalars, pointers, PODs}, etc.

Ok, noted.

glider added inline comments.Nov 14 2018, 5:18 AM
include/clang/Basic/Sanitizers.def
163

Um, this is unfortunate.
We don't strictly need this feature to live under -fsanitize= flag, although having the corresponding attribute could've been handy.

kcc added a comment.Nov 15 2018, 6:25 PM

I think you can safely abandon this change in favor of https://reviews.llvm.org/D54604 -- please join the review there.

jfb added a subscriber: jfb.Nov 16 2018, 10:24 AM
glider abandoned this revision.Nov 19 2018, 8:28 AM
emaste added a subscriber: emaste.Nov 20 2018, 1:33 PM