This is an archive of the discontinued LLVM Phabricator instance.

[asan] Implement "scribble" flag, which overwrites free'd memory with 0x55
ClosedPublic

Authored by kubamracek on Feb 17 2017, 10:19 AM.

Details

Summary

This patch implements a off-by-default flag called "scribble", which will overwrite free()'d memory with 0x55 bytes. This is to match the MallocScribble env var on macOS (see https://developer.apple.com/library/content/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html) and it's a helpful tool to better detect use-after-free bugs that happen in non-instrumented code. For example in Obj-C code, a lot of memory accesses happens within libobjc, which is not instrumented, and using "scribble=1" will make it crash much more reliably.

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek created this revision.Feb 17 2017, 10:19 AM
kubamracek updated this revision to Diff 88954.Feb 17 2017, 2:26 PM

Changing the testcase slightly. I realized that 0x55555555 can actually be a valid address in some platforms.

kcc edited edge metadata.Feb 17 2017, 2:42 PM

there is already max_malloc_fill_size and malloc_fill_byte
So to be consistent I suggest to have flags max_free_fill_size and free_fill_byte

I don't mind having max_free_fill_size=0x1000 by default.

filcab added inline comments.Feb 21 2017, 7:12 AM
lib/asan/asan_allocator.cc
533 ↗(On Diff #88954)

If you're re-using max_malloc_fill_size, please add to its description that it's also used for scribbling after deallocation.
Otherwise, having different flags for the deallocation scribbling looks good too.

lib/asan/asan_flags.inc
147 ↗(On Diff #88954)

"On deallocation..."? Since we also set it for delete/delete[]
I think the current phrasing is understandable, but seems better to phrase it more generally.

kubamracek updated this revision to Diff 93170.Mar 27 2017, 1:08 PM

Updating patch. This now uses the max_malloc_fill_size/max_free_fill_size and malloc_fill_byte/free_fill_byte flag names. We're also turning this on with the same env vars as Malloc Scribble on macOS does (https://developer.apple.com/library/content/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html).

kcc added inline comments.Mar 29 2017, 5:31 PM
lib/asan/asan_flags.cc
98 ↗(On Diff #93170)

Put this under if (SANITIZER_MAC) or something.
AFIACT, there is no such standard env var on other platforms.

test/asan/TestCases/scribble.cc
3 ↗(On Diff #93170)

add one more test with ASAN_OPTIONS instead of Mac-specific options.

kubamracek updated this revision to Diff 93431.Mar 29 2017, 6:03 PM
kubamracek marked 4 inline comments as done.
kcc added inline comments.Mar 29 2017, 6:04 PM
test/asan/TestCases/scribble.cc
3 ↗(On Diff #93431)

and now this test will fail on non-Mac, right?

kubamracek added inline comments.Mar 29 2017, 6:05 PM
test/asan/TestCases/scribble.cc
3 ↗(On Diff #93431)

Right, I thought this test was in TestCases/Darwin/.

kubamracek updated this revision to Diff 93434.Mar 29 2017, 6:10 PM
kcc accepted this revision.Mar 29 2017, 6:11 PM

LGTM

This revision is now accepted and ready to land.Mar 29 2017, 6:11 PM
This revision was automatically updated to reflect the committed changes.

Thanks for the review!