From 43a229697622b5933da1fdeb61d4eac2a2b7742c Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Tue, 17 Jul 2018 16:12:00 +0000 Subject: libFuzzer: prevent irrelevant strings from leaking into auto-dictionary This is a fix for bug 37047. https://bugs.llvm.org/show_bug.cgi?id=37047 Implemented by basically reversing the logic. Previously all strings were considered, with some operations excluded. Now strings are excluded by default, and only strings during the CB considered. Patch By: pdknsk Differential Revision: https://reviews.llvm.org/D48800 llvm-svn: 337296 --- compiler-rt/lib/fuzzer/FuzzerDefs.h | 8 ++------ compiler-rt/lib/fuzzer/FuzzerDictionary.h | 1 - compiler-rt/lib/fuzzer/FuzzerInternal.h | 1 - compiler-rt/lib/fuzzer/FuzzerLoop.cpp | 14 ++++++++------ compiler-rt/lib/fuzzer/FuzzerMutate.cpp | 1 - compiler-rt/lib/fuzzer/FuzzerTracePC.cpp | 18 ++++++++---------- compiler-rt/lib/fuzzer/FuzzerTracePC.h | 1 - compiler-rt/test/fuzzer/three-bytes.test | 4 ++-- 8 files changed, 20 insertions(+), 28 deletions(-) diff --git a/compiler-rt/lib/fuzzer/FuzzerDefs.h b/compiler-rt/lib/fuzzer/FuzzerDefs.h index 4868bc2d3ea..a35c7a181b7 100644 --- a/compiler-rt/lib/fuzzer/FuzzerDefs.h +++ b/compiler-rt/lib/fuzzer/FuzzerDefs.h @@ -176,12 +176,6 @@ typedef int (*UserCallback)(const uint8_t *Data, size_t Size); int FuzzerDriver(int *argc, char ***argv, UserCallback Callback); -struct ScopedDoingMyOwnMemOrStr { - ScopedDoingMyOwnMemOrStr() { DoingMyOwnMemOrStr++; } - ~ScopedDoingMyOwnMemOrStr() { DoingMyOwnMemOrStr--; } - static int DoingMyOwnMemOrStr; -}; - inline uint8_t Bswap(uint8_t x) { return x; } inline uint16_t Bswap(uint16_t x) { return __builtin_bswap16(x); } inline uint32_t Bswap(uint32_t x) { return __builtin_bswap32(x); } @@ -191,6 +185,8 @@ uint8_t *ExtraCountersBegin(); uint8_t *ExtraCountersEnd(); void ClearExtraCounters(); +extern bool RunningUserCallback; + } // namespace fuzzer #endif // LLVM_FUZZER_DEFS_H diff --git a/compiler-rt/lib/fuzzer/FuzzerDictionary.h b/compiler-rt/lib/fuzzer/FuzzerDictionary.h index 0077e2a990a..0d9d91bcd2f 100644 --- a/compiler-rt/lib/fuzzer/FuzzerDictionary.h +++ b/compiler-rt/lib/fuzzer/FuzzerDictionary.h @@ -33,7 +33,6 @@ public: } bool operator==(const FixedWord &w) const { - ScopedDoingMyOwnMemOrStr scoped_doing_my_own_mem_os_str; return Size == w.Size && 0 == memcmp(Data, w.Data, Size); } diff --git a/compiler-rt/lib/fuzzer/FuzzerInternal.h b/compiler-rt/lib/fuzzer/FuzzerInternal.h index 0eb42895548..bfc898248ad 100644 --- a/compiler-rt/lib/fuzzer/FuzzerInternal.h +++ b/compiler-rt/lib/fuzzer/FuzzerInternal.h @@ -118,7 +118,6 @@ private: uint8_t *CurrentUnitData = nullptr; std::atomic CurrentUnitSize; uint8_t BaseSha1[kSHA1NumBytes]; // Checksum of the base unit. - bool RunningCB = false; bool GracefulExitRequested = false; diff --git a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp index ba61c15f01b..a2d53ee48db 100644 --- a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp +++ b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp @@ -43,6 +43,8 @@ thread_local bool Fuzzer::IsMyThread; SharedMemoryRegion SMR; +bool RunningUserCallback = false; + // Only one Fuzzer per process. static Fuzzer *F; @@ -243,7 +245,7 @@ void Fuzzer::CrashCallback() { } void Fuzzer::ExitCallback() { - if (!RunningCB) + if (!RunningUserCallback) return; // This exit did not come from the user callback if (EF->__sanitizer_acquire_crash_state && !EF->__sanitizer_acquire_crash_state()) @@ -277,7 +279,7 @@ void Fuzzer::AlarmCallback() { if (!InFuzzingThread()) return; #endif - if (!RunningCB) + if (!RunningUserCallback) return; // We have not started running units yet. size_t Seconds = duration_cast(system_clock::now() - UnitStartTime).count(); @@ -451,9 +453,9 @@ void Fuzzer::CheckForUnstableCounters(const uint8_t *Data, size_t Size) { ScopedEnableMsanInterceptorChecks S; UnitStartTime = system_clock::now(); TPC.ResetMaps(); - RunningCB = true; + RunningUserCallback = true; CB(Data, Size); - RunningCB = false; + RunningUserCallback = false; UnitStopTime = system_clock::now(); }; @@ -558,9 +560,9 @@ void Fuzzer::ExecuteCallback(const uint8_t *Data, size_t Size) { AllocTracer.Start(Options.TraceMalloc); UnitStartTime = system_clock::now(); TPC.ResetMaps(); - RunningCB = true; + RunningUserCallback = true; int Res = CB(DataCopy, Size); - RunningCB = false; + RunningUserCallback = false; UnitStopTime = system_clock::now(); (void)Res; assert(Res == 0); diff --git a/compiler-rt/lib/fuzzer/FuzzerMutate.cpp b/compiler-rt/lib/fuzzer/FuzzerMutate.cpp index 865e598fdc8..6f6ce075ab9 100644 --- a/compiler-rt/lib/fuzzer/FuzzerMutate.cpp +++ b/compiler-rt/lib/fuzzer/FuzzerMutate.cpp @@ -195,7 +195,6 @@ DictionaryEntry MutationDispatcher::MakeDictionaryEntryFromCMP( const void *Arg1Mutation, const void *Arg2Mutation, size_t ArgSize, const uint8_t *Data, size_t Size) { - ScopedDoingMyOwnMemOrStr scoped_doing_my_own_mem_os_str; bool HandleFirst = Rand.RandBool(); const void *ExistingBytes, *DesiredBytes; Word W; diff --git a/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp b/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp index ed920b8e59a..e61e9116bbd 100644 --- a/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp +++ b/compiler-rt/lib/fuzzer/FuzzerTracePC.cpp @@ -39,8 +39,6 @@ namespace fuzzer { TracePC TPC; -int ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr; - uint8_t *TracePC::Counters() const { return __sancov_trace_pc_guard_8bit_counters; } @@ -608,7 +606,7 @@ void __sanitizer_cov_trace_gep(uintptr_t Idx) { ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_memcmp(void *caller_pc, const void *s1, const void *s2, size_t n, int result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; if (result == 0) return; // No reason to mutate. if (n <= 1) return; // Not interesting. fuzzer::TPC.AddValueForMemcmp(caller_pc, s1, s2, n, /*StopAtZero*/false); @@ -617,7 +615,7 @@ void __sanitizer_weak_hook_memcmp(void *caller_pc, const void *s1, ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_strncmp(void *caller_pc, const char *s1, const char *s2, size_t n, int result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; if (result == 0) return; // No reason to mutate. size_t Len1 = fuzzer::InternalStrnlen(s1, n); size_t Len2 = fuzzer::InternalStrnlen(s2, n); @@ -630,7 +628,7 @@ void __sanitizer_weak_hook_strncmp(void *caller_pc, const char *s1, ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_strcmp(void *caller_pc, const char *s1, const char *s2, int result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; if (result == 0) return; // No reason to mutate. size_t N = fuzzer::InternalStrnlen2(s1, s2); if (N <= 1) return; // Not interesting. @@ -640,35 +638,35 @@ void __sanitizer_weak_hook_strcmp(void *caller_pc, const char *s1, ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_strncasecmp(void *called_pc, const char *s1, const char *s2, size_t n, int result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; return __sanitizer_weak_hook_strncmp(called_pc, s1, s2, n, result); } ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_strcasecmp(void *called_pc, const char *s1, const char *s2, int result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; return __sanitizer_weak_hook_strcmp(called_pc, s1, s2, result); } ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_strstr(void *called_pc, const char *s1, const char *s2, char *result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; fuzzer::TPC.MMT.Add(reinterpret_cast(s2), strlen(s2)); } ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_strcasestr(void *called_pc, const char *s1, const char *s2, char *result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; fuzzer::TPC.MMT.Add(reinterpret_cast(s2), strlen(s2)); } ATTRIBUTE_INTERFACE ATTRIBUTE_NO_SANITIZE_MEMORY void __sanitizer_weak_hook_memmem(void *called_pc, const void *s1, size_t len1, const void *s2, size_t len2, void *result) { - if (fuzzer::ScopedDoingMyOwnMemOrStr::DoingMyOwnMemOrStr) return; + if (!fuzzer::RunningUserCallback) return; fuzzer::TPC.MMT.Add(reinterpret_cast(s2), len2); } } // extern "C" diff --git a/compiler-rt/lib/fuzzer/FuzzerTracePC.h b/compiler-rt/lib/fuzzer/FuzzerTracePC.h index 424c20722a1..416b28427e0 100644 --- a/compiler-rt/lib/fuzzer/FuzzerTracePC.h +++ b/compiler-rt/lib/fuzzer/FuzzerTracePC.h @@ -180,7 +180,6 @@ private: std::pair FocusFunction = {-1, -1}; // Module and PC IDs. - ValueBitMap ValueProfileMap; uintptr_t InitialStack; }; diff --git a/compiler-rt/test/fuzzer/three-bytes.test b/compiler-rt/test/fuzzer/three-bytes.test index 242be49e422..0b2187552cf 100644 --- a/compiler-rt/test/fuzzer/three-bytes.test +++ b/compiler-rt/test/fuzzer/three-bytes.test @@ -1,8 +1,8 @@ Tests -use_value_profile=2 (alternative VP metric). RUN: %cpp_compiler %S/ThreeBytes.cpp -o %t -RUN: %run %t -seed=1 -runs=100000 -RUN: %run %t -seed=1 -runs=100000 -use_value_profile=1 +RUN: %run %t -seed=1 -runs=30000 +RUN: %run %t -seed=1 -runs=30000 -use_value_profile=1 RUN: not %run %t -seed=1 -runs=1000000 -use_value_profile=2 2>&1 | FileCheck %s CHECK: Test unit written -- cgit v1.2.3