[compiler-rt] bb3a3da - [DFSan] Don't unmap during dfsan_flush().

[Matt Morehouse via llvm-commits]

Author: Matt Morehouse
Date: 2020-08-14T11:43:49-07:00
New Revision: bb3a3da38d01cd5639b502ec3979d201d59f1950

URL: https://github.com/llvm/llvm-project/commit/bb3a3da38d01cd5639b502ec3979d201d59f1950
DIFF: https://github.com/llvm/llvm-project/commit/bb3a3da38d01cd5639b502ec3979d201d59f1950.diff

LOG: [DFSan] Don't unmap during dfsan_flush().

Unmapping and remapping is dangerous since another thread could touch
the shadow memory while it is unmapped.  But there is really no need to
unmap anyway, since mmap(MAP_FIXED) will happily clobber the existing
mapping with zeroes.  This is thread-safe since the mmap() is done under
the same kernel lock as page faults are done.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D85947

Added: 
    compiler-rt/test/dfsan/threaded_flush.c

Modified: 
    compiler-rt/include/sanitizer/dfsan_interface.h
    compiler-rt/lib/dfsan/dfsan.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/include/sanitizer/dfsan_interface.h b/compiler-rt/include/sanitizer/dfsan_interface.h
index 81546e5df71a..18b2c81a6023 100644
--- a/compiler-rt/include/sanitizer/dfsan_interface.h
+++ b/compiler-rt/include/sanitizer/dfsan_interface.h
@@ -80,9 +80,11 @@ dfsan_label dfsan_has_label_with_desc(dfsan_label label, const char *desc);
 size_t dfsan_get_label_count(void);
 
 /// Flushes the DFSan shadow, i.e. forgets about all labels currently associated
-/// with the application memory. Will work only if there are no other
-/// threads executing DFSan-instrumented code concurrently.
-/// Use this call to start over the taint tracking within the same procces.
+/// with the application memory.  Use this call to start over the taint tracking
+/// within the same process.
+///
+/// Note: If another thread is working with tainted data during the flush, that
+/// taint could still be written to shadow after the flush.
 void dfsan_flush(void);
 
 /// Sets a callback to be invoked on calls to write().  The callback is invoked

diff  --git a/compiler-rt/lib/dfsan/dfsan.cpp b/compiler-rt/lib/dfsan/dfsan.cpp
index 678f6c1183e0..b2f474890e1d 100644
--- a/compiler-rt/lib/dfsan/dfsan.cpp
+++ b/compiler-rt/lib/dfsan/dfsan.cpp
@@ -428,7 +428,6 @@ static void dfsan_fini() {
 }
 
 extern "C" void dfsan_flush() {
-  UnmapOrDie((void*)ShadowAddr(), UnusedAddr() - ShadowAddr());
   if (!MmapFixedNoReserve(ShadowAddr(), UnusedAddr() - ShadowAddr()))
     Die();
 }

diff  --git a/compiler-rt/test/dfsan/threaded_flush.c b/compiler-rt/test/dfsan/threaded_flush.c
new file mode 100644
index 000000000000..66f41dbe96ca
--- /dev/null
+++ b/compiler-rt/test/dfsan/threaded_flush.c
@@ -0,0 +1,36 @@
+// Tests that doing dfsan_flush() while another thread is executing doesn't
+// segfault.
+// RUN: %clang_dfsan %s -o %t && %run %t
+#include <assert.h>
+#include <pthread.h>
+#include <sanitizer/dfsan_interface.h>
+#include <stdlib.h>
+
+static unsigned char GlobalBuf[4096];
+static int ShutDownThread;
+static int StartFlush;
+
+// Access GlobalBuf continuously, causing its shadow to be touched as well.
+// When main() calls dfsan_flush(), no segfault should be triggered.
+static void *accessGlobalInBackground(void *Arg) {
+  __atomic_store_n(&StartFlush, 1, __ATOMIC_RELEASE);
+
+  while (!__atomic_load_n(&ShutDownThread, __ATOMIC_ACQUIRE))
+    for (unsigned I = 0; I < sizeof(GlobalBuf); ++I)
+      ++GlobalBuf[I];
+
+  return NULL;
+}
+
+int main() {
+  pthread_t Thread;
+  pthread_create(&Thread, NULL, accessGlobalInBackground, NULL);
+  while (!__atomic_load_n(&StartFlush, __ATOMIC_ACQUIRE))
+    ; // Spin
+
+  dfsan_flush();
+
+  __atomic_store_n(&ShutDownThread, 1, __ATOMIC_RELEASE);
+  pthread_join(Thread, NULL);
+  return 0;
+}