Page MenuHomePhabricator

[libFuzzer] Make -fsanitize=memory,fuzzer work.
ClosedPublic

Authored by morehouse on Jul 3 2018, 12:27 PM.

Details

Summary

This patch allows libFuzzer to fuzz applications instrumented with MSan
without recompiling libFuzzer with MSan instrumentation.

Fixes https://github.com/google/sanitizers/issues/958.

Diff Detail

Event Timeline

morehouse created this revision.Jul 3 2018, 12:27 PM
kcc added inline comments.Jul 3 2018, 1:23 PM
compiler-rt/lib/fuzzer/FuzzerLoop.cpp
182 ↗(On Diff #153955)

why do you need this here?

compiler-rt/test/fuzzer/msan.test
17 ↗(On Diff #153955)

it would be nice to have a separate msan-ish test with a real msan-ish bug (not a buffer overflow)

morehouse added inline comments.Jul 3 2018, 2:43 PM
compiler-rt/lib/fuzzer/FuzzerLoop.cpp
182 ↗(On Diff #153955)

DumpCurrentUnit is called from most error callbacks. When that happens, interceptor checks are still active.

Since we eventually call fopen to write out the current unit, we need to disable interceptor checks again here.

kcc added inline comments.Jul 3 2018, 2:54 PM
compiler-rt/lib/fuzzer/FuzzerLoop.cpp
182 ↗(On Diff #153955)

ACK. please make it a scoped operator (a class with a CTOR calling disable and DTOR calling enable).

morehouse updated this revision to Diff 154011.Jul 3 2018, 4:08 PM
  • Merge branch 'memfuzz' into memfuzz2
  • Sync with parent patch.
  • Added use-after-dtor test.
  • Disable/enable MSan interceptors via scoped class.
morehouse marked 2 inline comments as done.Jul 3 2018, 4:09 PM
kcc added inline comments.Jul 3 2018, 4:15 PM
compiler-rt/lib/fuzzer/FuzzerDriver.cpp
541 ↗(On Diff #154011)

errr. this sounds like an overkill.
If you never destruct this, then just call __msan_scoped_disable_interceptor_checks

compiler-rt/lib/fuzzer/FuzzerInternal.h
155 ↗(On Diff #154011)

you only ever need ScopedEnable, right?
never ScopedDisable

compiler-rt/lib/fuzzer/FuzzerLoop.cpp
182 ↗(On Diff #154011)

Do you need this extra scope here?

519 ↗(On Diff #154011)

why Data, not DataCopy?

compiler-rt/test/fuzzer/msan.test
18 ↗(On Diff #154011)

also add one test that would break if you accidentally disable and never enable back the interceptor checks

morehouse added inline comments.Jul 3 2018, 5:11 PM
compiler-rt/lib/fuzzer/FuzzerInternal.h
155 ↗(On Diff #154011)

Technically yes, we could avoid re-enabling after the write in DumpCurrentUnit, since DumpCurrentUnit is currently only ever called right before crashing. But I'm not sure we want to bank on that...

Since DumpCurrentUnit doesn't crash itself, a future change could end up calling it outside of a crash handler. If that happens, we would have a tricky bug where MSan's interceptor checks are permanently disabled.

morehouse updated this revision to Diff 154034.Jul 3 2018, 6:00 PM
morehouse marked 4 inline comments as done.
  • Remove global from FuzzerDriver.cpp.
  • Remove unnecessary scope.
  • Unpoison DataCopy.
  • Add strlen test.
kcc accepted this revision.Jul 9 2018, 3:29 PM

LGTM

Please also update the docs (http://llvm.org/docs/LibFuzzer.html) once we are confident that msan+libfuzzer works out of the box

compiler-rt/test/fuzzer/UninitializedStrlen.cpp
1 ↗(On Diff #154034)

here and below make sure the test has the proper header.

This revision is now accepted and ready to land.Jul 9 2018, 3:29 PM
kcc added a comment.Jul 9 2018, 3:30 PM
In D48891#1156587, @kcc wrote:

LGTM

Please also update the docs (http://llvm.org/docs/LibFuzzer.html) once we are confident that msan+libfuzzer works out of the box

I mean, in a separate patch.

morehouse updated this revision to Diff 154733.Jul 9 2018, 4:48 PM
  • Update test headers.
morehouse updated this revision to Diff 154734.Jul 9 2018, 4:50 PM
  • Correct diffbase.
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: Restricted Project. · View Herald TranscriptJul 9 2018, 4:56 PM
Dor1s added a subscriber: Dor1s.Jul 16 2018, 7:05 AM

Hey Matt, this new test seems to be failing for me locally. I've run ninja check-msan, but it doesn't help. I assume that the problem is on my side, do you have any clue what I can be missing?

The failure is that it doesn't print BINGO, as it seems to be crashing before that:

#4523	REDUCE cov: 6 ft: 6 corp: 5/9b lim: 6 exec/s: 0 rss: 38Mb L: 3/3 MS: 3 CopyPart-EraseBytes-InsertByte-
==103360==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x53ca2b in std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::__put_character_sequence<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, char const*, unsigned long) <...>llvm/build/bin/../include/c++/v1/ostream:722:13
    #1 0x53c4c1 in LLVMFuzzerTestOneInput <...>llvm/llvm/projects/compiler-rt/test/fuzzer/SimpleTest.cpp:21:19
    #2 0x4528ac in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) <...>llvm/llvm/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:531
    #3 0x459cd6 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) <...>llvm/llvm/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:452
    #4 0x459cd6 in fuzzer::Fuzzer::MutateAndTestOne() <...>llvm/llvm/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:664
    #5 0x45d83f in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<std::__Fuzzer::basic_string<char, std::__Fuzzer::char_traits<char>, std::__Fuzzer::allocator<char> >, fuzzer::fuzzer_allocator<std::__Fuzzer::basic_string<char, std::__Fuzzer::char_traits<char>, std::__Fuzzer::allocator<char> > > > const&) <...>llvm/llvm/projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:794
    #6 0x44e233 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) <...>llvm/llvm/projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:760
    #7 0x421272 in main <...>llvm/llvm/projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20
    #8 0x7f9a300502b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
    #9 0x4212a9 in _start (<...>llvm/build/projects/compiler-rt/test/fuzzer/Output/msan.test.tmp+0x4212a9)

SUMMARY: MemorySanitizer: use-of-uninitialized-value <...>llvm/build/bin/../include/c++/v1/ostream:722:13 in std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::__put_character_sequence<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, char const*, unsigned long)
Exiting
MS: 1 ChangeBit-; base unit: ba5baccb9bbf5a3e04d647914437de87a1fae521
0x48,0x69,0x21,
Hi!
artifact_prefix='./'; Test unit written to ./crash-c0a0ad26a634840c67a210fefdda76577b03a111
Base64: SGkh

Hmm... Seems to be crashing in std::cout <<, but nothing there should be uninitialized. Also the bots are all green.

What revision are you at? Is the libFuzzer revision in sync with the rest of LLVM / compiler-rt?

Dor1s added a comment.Jul 16 2018, 9:53 AM

I'm on r337187. Yeah, all the repos seem to be updated.

Looks like my local build isn't linking against a special libc++ mentioned here: https://github.com/google/oss-fuzz/issues/1556#issuecomment-399818430

I suspected that my cmake config could be wrong, so tried the following one copied from a buildbot:

cmake -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=OFF \
    -DLLVM_PARALLEL_LINK_JOBS=8 -DLIBFUZZER_ENABLE_TESTS=ON

but the issue still persists.

Make sure you have the libcxx sources checked out (I think under llvm/projects). I think that should fix it.

Also, sync to ToT since I just added a test dependency for MSan.

But now that you bring this up, we might want to create a libcxx feature that the MSan test can require.

Good call, but I have libcxx synced to ToT already :(

However, it's very likely to be the root cause:

$ ldd <...>/Projects/llvm/build/projects/compiler-rt/test/fuzzer/Output/msan.test.tmp
	linux-vdso.so.1 (0x00007ffe87f6f000)
	libc++.so.1 => /usr/lib/x86_64-linux-gnu/libc++.so.1 (0x00007f613884d000)
	libc++abi.so.1 => <...>/Projects/llvm/build/./lib/libc++abi.so.1 (0x00007f6138620000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f613831c000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f61380ff000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f6137ef7000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f6137cf3000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f6137adb000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f613773c000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f6138b53000)

Let me build everything completely from scratch. I'm sure something is messed up with my cmake config / cmake files in the build dir. I'll post an update. Thanks for your help!

After more thinking, the private libc++ probably isn't the issue here since only libFuzzer itself gets linked with that. The fuzz target is linked with whatever C++ standard library your system is configured to use.

On my machine, that's libstdc++, but yours is libc++. Perhaps libc++ implements cout << ... differently, therefore causing the false positive... Let me know if the rebuild fixes it. Otherwise, I'll take a closer look.

kcc added a comment.Jul 16 2018, 12:20 PM

Maybe just change the msan-ish test to not use STL

Dor1s added a comment.Jul 16 2018, 1:02 PM

Let me know if the rebuild fixes it. Otherwise, I'll take a closer look.

Didn't help. Also, the stacktrace points to the libcxx source file, even though ldd shows the system libc++ being used:

mmoroz@mmoroz2:~/Projects/llvm/build$ sha1sum include/c++/v1/ostream
99ce0b9ca18a5c9e4c3d88b35fbae6416135ae9a  include/c++/v1/ostream
mmoroz@mmoroz2:~/Projects/llvm/build$ sha1sum ../llvm/projects/libcxx/include/ostream 
99ce0b9ca18a5c9e4c3d88b35fbae6416135ae9a  ../llvm/projects/libcxx/include/ostream

Try again after syncing past r337224. Hopefully that fixes your issue.

Dor1s added a comment.Jul 17 2018, 7:20 AM

Try again after syncing past r337224. Hopefully that fixes your issue.

Yay, everything works now. Thank you, guys!