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

[morehouse]

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.

[kcc]

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]

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]

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]

• Merge branch 'memfuzz' into memfuzz2
• Sync with parent patch.
• Added use-after-dtor test.
• Disable/enable MSan interceptors via scoped class.

[kcc]

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]

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]

• Remove global from FuzzerDriver.cpp.
• Remove unnecessary scope.
• Unpoison DataCopy.
• Add strlen test.

[kcc]

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.

[kcc]

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]

• Update test headers.

[morehouse]

• Correct diffbase.

[Dor1s]

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

[morehouse]

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]

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.

[morehouse]

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.

[Dor1s]

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)

[Dor1s]

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!

[morehouse]

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]

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

[Dor1s]

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

[morehouse]

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

[Dor1s]

In D48891#1164431, @morehouse wrote:

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

Yay, everything works now. Thank you, guys!

lib/fuzzer/FuzzerDriver.cpp

 
 int FuzzerDriver(int *argc, char ***argv, UserCallback Callback) {
   using namespace fuzzer;
   assert(argc && argv && "Argument pointers cannot be nullptr");
   std::string Argv0((*argv)[0]);
   EF = new ExternalFunctions();
   if (EF->LLVMFuzzerInitialize)
     EF->LLVMFuzzerInitialize(argc, argv);
+  if (EF->__msan_scoped_disable_interceptor_checks)
+    EF->__msan_scoped_disable_interceptor_checks();
   const Vector<std::string> Args(*argv, *argv + *argc);
   assert(!Args.empty());
   ProgName = new std::string(Args[0]);
   if (Argv0 != *ProgName) {
     Printf("ERROR: argv[0] has been modified in LLVMFuzzerInitialize\n");
     exit(1);
   }
   ParseFlags(Args);

lib/fuzzer/FuzzerExtFunctions.def

EXT_FUNC(__sanitizer_symbolize_pc, void,
          (void *, const char *fmt, char *out_buf, size_t out_buf_size), false);
 EXT_FUNC(__sanitizer_get_module_and_offset_for_pc, int,
          (void *pc, char *module_path,
          size_t module_path_len,void **pc_offset), false);
 EXT_FUNC(__sanitizer_set_death_callback, void, (void (*)(void)), true);
 EXT_FUNC(__sanitizer_set_report_fd, void, (void*), false);
 EXT_FUNC(__sanitizer_dump_coverage, void, (const uintptr_t *, uintptr_t),
          false);
+EXT_FUNC(__msan_scoped_disable_interceptor_checks, void, (), false);
+EXT_FUNC(__msan_scoped_enable_interceptor_checks, void, (), false);
+EXT_FUNC(__msan_unpoison, void, (const volatile void *, size_t size), false);

lib/fuzzer/FuzzerInternal.h

private:
   size_t TmpMaxMutationLen = 0;
 
   Vector<uint32_t> UniqFeatureSetTmp;
 
   // Need to know our own thread.
   static thread_local bool IsMyThread;
 };
 
+struct ScopedEnableMsanInterceptorChecks {
+  ScopedEnableMsanInterceptorChecks() {
+    if (EF->__msan_scoped_enable_interceptor_checks)
+      EF->__msan_scoped_enable_interceptor_checks();
+  }
+  ~ScopedEnableMsanInterceptorChecks() {
+    if (EF->__msan_scoped_disable_interceptor_checks)
+      EF->__msan_scoped_disable_interceptor_checks();
+  }
+};
+
+struct ScopedDisableMsanInterceptorChecks {
+  ScopedDisableMsanInterceptorChecks() {
+    if (EF->__msan_scoped_disable_interceptor_checks)
+      EF->__msan_scoped_disable_interceptor_checks();
+  }
+  ~ScopedDisableMsanInterceptorChecks() {
+    if (EF->__msan_scoped_enable_interceptor_checks)
+      EF->__msan_scoped_enable_interceptor_checks();
+  }
+};
+
 } // namespace fuzzer
 
 #endif // LLVM_FUZZER_INTERNAL_H

lib/fuzzer/FuzzerLoop.cpp

 void Fuzzer::StaticDeathCallback() {
   assert(F);
   F->DeathCallback();
 }
 
 void Fuzzer::DumpCurrentUnit(const char *Prefix) {
   if (!CurrentUnitData)
     return; // Happens when running individual inputs.
+  ScopedDisableMsanInterceptorChecks S;
   MD.PrintMutationSequence();
   Printf("; base unit: %s\n", Sha1ToString(BaseSha1).c_str());
   size_t UnitSize = CurrentUnitSize;
   if (UnitSize <= kMaxUnitSizeToPrint) {
     PrintHexArray(CurrentUnitData, UnitSize, "\n");
     PrintASCII(CurrentUnitData, UnitSize, "\n");
   }
   WriteUnitToFileWithPrefix({CurrentUnitData, CurrentUnitData + UnitSize},
void Fuzzer::ExecuteCallback(const uint8_t *Data, size_t Size) {
   TotalNumberOfRuns++;
   assert(InFuzzingThread());
   if (SMR.IsClient())
     SMR.WriteByteArray(Data, Size);
   // We copy the contents of Unit into a separate heap buffer
   // so that we reliably find buffer overflows in it.
   uint8_t *DataCopy = new uint8_t[Size];
   memcpy(DataCopy, Data, Size);
+  if (EF->__msan_unpoison)
+    EF->__msan_unpoison(DataCopy, Size);
   if (CurrentUnitData && CurrentUnitData != Data)
     memcpy(CurrentUnitData, Data, Size);
   CurrentUnitSize = Size;
+  {
+    ScopedEnableMsanInterceptorChecks S;
     AllocTracer.Start(Options.TraceMalloc);
     UnitStartTime = system_clock::now();
     TPC.ResetMaps();
     RunningCB = true;
     int Res = CB(DataCopy, Size);
     RunningCB = false;
     UnitStopTime = system_clock::now();
     (void)Res;
     assert(Res == 0);
     HasMoreMallocsThanFrees = AllocTracer.Stop();
+  }
   if (!LooseMemeq(DataCopy, Data, Size))
     CrashOnOverwrittenData();
   CurrentUnitSize = 0;
   delete[] DataCopy;
 }
 
 void Fuzzer::WriteToOutputCorpus(const Unit &U) {
   if (Options.OnlyASCII)

test/fuzzer/UninitializedStrlen.cpp

+#include <cstdint>
+#include <cstring>
+
+volatile size_t Sink;
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
+  if (Size < 4) return 0;
+  if (Data[0] == 'F' && Data[1] == 'U' && Data[2] == 'Z' && Data[3] == 'Z') {
+    char uninit[7];
+    Sink = strlen(uninit);
+  }
+  return 0;
+}
+

test/fuzzer/UseAfterDtor.cpp

+#include <cstdint>
+#include <cstdio>
+
+struct Simple {
+  int x_;
+  Simple() {
+    x_ = 5;
+  }
+  ~Simple() {
+    x_ += 1;
+  }
+};
+
+Simple *volatile SimpleSink;
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
+  if (Size < 4) return 0;
+  if (Data[0] == 'F' && Data[1] == 'U' && Data[2] == 'Z' && Data[3] == 'Z') {
+    {
+      Simple S;
+      SimpleSink = &S;
+    }
+    if (SimpleSink->x_) fprintf(stderr, "Failed to catch use-after-dtor\n");
+  }
+  return 0;
+}
+

test/fuzzer/lit.cfg

if sys.platform.startswith('linux'):
   config.available_features.add('linux')
 else:
   lit_config.note('linux feature unavailable')
 
 config.substitutions.append(('%build_dir', config.cmake_binary_dir))
 libfuzzer_src_root = os.path.join(config.compiler_rt_src_root, "lib", "fuzzer")
 config.substitutions.append(('%libfuzzer_src', libfuzzer_src_root))
 
-def generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True):
+def generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True, msan_enabled=False):
   compiler_cmd = config.clang
   extra_cmd = config.target_flags
   if config.clang and config.stdlib == 'libc++':
     link_cmd = '-stdlib=libc++ -Wl,-rpath=%s' % config.runtime_library_dir
   elif config.clang and config.stdlib == 'static-libc++':
     link_cmd = '-stdlib=libc++ -lc++abi -static-libstdc++ -Wl,-rpath=%s' % (
         config.runtime_library_dir)
   elif any(x in config.target_triple for x in ('darwin', 'freebsd')):
     link_cmd = '-lc++'
   else:
     link_cmd = '-lstdc++'
 
   std_cmd = '--driver-mode=g++ -std=c++11' if is_cpp else ''
+  if msan_enabled:
+    sanitizers = ['memory']
+  else:
     sanitizers = ['address']
   if fuzzer_enabled:
     sanitizers.append('fuzzer')
   sanitizers_cmd = ('-fsanitize=%s' % ','.join(sanitizers))
   return " ".join([
     compiler_cmd,
     std_cmd,
     link_cmd,
     "-O2 -gline-tables-only",

 config.substitutions.append(('%no_fuzzer_cpp_compiler',
       generate_compiler_cmd(is_cpp=True, fuzzer_enabled=False)
       ))
 
 config.substitutions.append(('%no_fuzzer_c_compiler',
       generate_compiler_cmd(is_cpp=False, fuzzer_enabled=False)
       ))
 
+config.substitutions.append(('%msan_compiler',
+      generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True, msan_enabled=True)
+      ))
+
 if config.host_os == 'Darwin':
   if config.target_arch in ["x86_64", "x86_64h"]:
     config.parallelism_group = "darwin-64bit-sanitizer"
   elif config.apple_platform != "osx" and not config.apple_platform.endswith("sim"):
     config.parallelism_group = "darwin-ios-device-sanitizer"

test/fuzzer/msan.test

+RUN: %msan_compiler %S/SimpleTest.cpp -o %t
+RUN: not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s --check-prefix=NO-REPORT
+
+RUN: %msan_compiler %S/SimpleCmpTest.cpp -o %t
+RUN: not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s --check-prefix=NO-REPORT
+
+RUN: %msan_compiler %S/MemcmpTest.cpp -o %t
+RUN: not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s --check-prefix=NO-REPORT
+
+RUN: %msan_compiler %S/StrcmpTest.cpp -o %t
+RUN: not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s --check-prefix=NO-REPORT
+
+NO-REPORT-NOT: MemorySanitizer
+NO-REPORT: BINGO
+
+
+RUN: %msan_compiler %S/UseAfterDtor.cpp -o %t
+RUN: MSAN_OPTIONS=poison_in_dtor=1 not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s --check-prefix=REPORT
+
+RUN: %msan_compiler %S/UninitializedStrlen.cpp -o %t
+RUN: not %run %t -seed=1 -runs=10000000 2>&1 | FileCheck %s --check-prefix=REPORT
+
+REPORT: MemorySanitizer: use-of-uninitialized-value