[sanitizer] Bail out with warning if user dlopens shared library with RTLD_DEEPBIND flag

[m.ostapenko]

Summary

People keep hitting on spurious failures in malloc/free routines when using sanitizers with shared libraries dlopened with RTLD_DEEPBIND (see https://github.com/google/sanitizers/issues/611 for details).
Let's check for this flag and bail out with warning message instead of failing in random places.

[ygribov]

lib/sanitizer_common/sanitizer_common.cc

485 ↗

(On Diff #90200)

Does it make sense to print backtrace?

test/sanitizer_common/TestCases/Posix/deepbind.cc

1 ↗

(On Diff #90200)

Do you need shlib at all? You don't care if dlopen succeeds so can call it with arbitrary path.

4 ↗

(On Diff #90200)

Android does not have it?

18 ↗

(On Diff #90200)

You can just return 0 after this.

[kubamracek]

lib/sanitizer_common/sanitizer_common.cc

480 ↗

(On Diff #90200)

This will not compile on macOS (RTLD_DEEPBIND is not defined there).

[m.ostapenko]

lib/sanitizer_common/sanitizer_common.cc

480 ↗

(On Diff #90200)

Thanks, will move this to sanitizer_linux. cc.

485 ↗

(On Diff #90200)

Perhaps. At least we should print library name here.

test/sanitizer_common/TestCases/Posix/deepbind.cc

1 ↗

(On Diff #90200)

Just wanted to make sure that without deepbind everything works as expected. I can drop this for sure.

4 ↗

(On Diff #90200)

AFAIK Android doesn't intercept dlopen.

[m.ostapenko]

Updating.

[m.ostapenko]

lib/sanitizer_common/sanitizer_common.cc

485 ↗

(On Diff #90200)

sanitizer_common doesn't have unified stacktrace printing routine (e.g. GET_STACK_TRACE_FATAL), every tool defines it by itself. Thus I don't print backtrace here to avoid unnecessary complexity.

[vitalybuka]

lib/sanitizer_common/sanitizer_linux.cc

1472 ↗

(On Diff #90291)

I'd prefer we have flags to ignore this. But according kcc@ on the bug it's overkill, so LGTM

[thakis]

We have a test in chromium that loads a small so file and calls some of the functions therein. There are no allocations in the so file. The test used to run fine under tsan, now it fails with the error message you added. I'll disable the test under tsan, but maybe others out there where also calling non-allocating so's without problems before this change.

[m.ostapenko]

In D30504#711277, @thakis wrote:

We have a test in chromium that loads a small so file and calls some of the functions therein. There are no allocations in the so file. The test used to run fine under tsan, now it fails with the error message you added. I'll disable the test under tsan, but maybe others out there where also calling non-allocating so's without problems before this change.

Oh, sorry. Perhaps we can remove Die()? The check still should fire a warning IMHO.

compiler-rt/trunk/lib/asan/asan_interceptors.cc

 // can be slow.
 #define COMMON_INTERCEPTOR_SET_PTHREAD_NAME(ctx, thread, name) \
   do {                                                         \
   } while (false)
 #define COMMON_INTERCEPTOR_BLOCK_REAL(name) REAL(name)
 // Strict init-order checking is dlopen-hostile:
 // https://github.com/google/sanitizers/issues/178
 #define COMMON_INTERCEPTOR_ON_DLOPEN(filename, flag)                           \
-  if (flags()->strict_init_order) {                                            \
-    StopInitOrderChecking();                                                   \
-  }
+  do {                                                                         \
+    if (flags()->strict_init_order)                                            \
+      StopInitOrderChecking();                                                 \
+    CheckNoDeepBind(filename, flag);                                           \
+  } while (false)
 #define COMMON_INTERCEPTOR_ON_EXIT(ctx) OnExit()
 #define COMMON_INTERCEPTOR_LIBRARY_LOADED(filename, handle) \
   CoverageUpdateMapping()
 #define COMMON_INTERCEPTOR_LIBRARY_UNLOADED() CoverageUpdateMapping()
 #define COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED (!asan_inited)
 #define COMMON_INTERCEPTOR_GET_TLS_RANGE(begin, end)                           \
   if (AsanThread *t = GetCurrentThread()) {                                    \
     *begin = t->tls_begin();                                                   \

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

struct StackDepotStats {
   uptr n_uniq_ids;
   uptr allocated;
 };
 
 // The default value for allocator_release_to_os_interval_ms common flag to
 // indicate that sanitizer allocator should not attempt to release memory to OS.
 const s32 kReleaseToOSIntervalNever = -1;
 
+void CheckNoDeepBind(const char *filename, int flag);
+
 }  // namespace __sanitizer
 
 inline void *operator new(__sanitizer::operator_new_size_type size,
                           __sanitizer::LowLevelAllocator &alloc) {
   return alloc.Allocate(size);
 }
 
 #endif  // SANITIZER_COMMON_H

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc

 #define COMMON_INTERCEPTOR_READ_STRING_OF_LEN(ctx, s, len, n)       \
     COMMON_INTERCEPTOR_READ_RANGE((ctx), (s),                       \
       common_flags()->strict_string_checks ? (len) + 1 : (n) )
 
 #define COMMON_INTERCEPTOR_READ_STRING(ctx, s, n)                   \
     COMMON_INTERCEPTOR_READ_STRING_OF_LEN((ctx), (s), REAL(strlen)(s), (n))
 
 #ifndef COMMON_INTERCEPTOR_ON_DLOPEN
-#define COMMON_INTERCEPTOR_ON_DLOPEN(filename, flag) {}
+#define COMMON_INTERCEPTOR_ON_DLOPEN(filename, flag) \
+  CheckNoDeepBind(filename, flag);
 #endif
 
 #ifndef COMMON_INTERCEPTOR_GET_TLS_RANGE
 #define COMMON_INTERCEPTOR_GET_TLS_RANGE(begin, end) *begin = *end = 0;
 #endif
 
 #ifndef COMMON_INTERCEPTOR_ACQUIRE
 #define COMMON_INTERCEPTOR_ACQUIRE(ctx, u) {}

compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc

 }
 
 void MaybeReexec() {
   // No need to re-exec on Linux.
 }
 
 void PrintModuleMap() { }
 
+void CheckNoDeepBind(const char *filename, int flag) {
+  if (flag & RTLD_DEEPBIND) {
+    Report(
+        "You are trying to dlopen a %s shared library with RTLD_DEEPBIND flag"
+        " which is incompatibe with sanitizer runtime "
+        "(see https://github.com/google/sanitizers/issues/611 for details"
+        "). If you want to run %s library under sanitizers please remove "
+        "RTLD_DEEPBIND from dlopen flags.\n",
+        filename, filename);
+    Die();
+  }
+}
+
 uptr FindAvailableMemoryRange(uptr size, uptr alignment, uptr left_padding) {
   UNREACHABLE("FindAvailableMemoryRange is not available");
   return 0;
 }
 
 } // namespace __sanitizer
 
 #endif // SANITIZER_FREEBSD || SANITIZER_LINUX

compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc

for (uptr i = 0; i < modules.size(); ++i) {
     FormatUUID(uuid_str, sizeof(uuid_str), modules[i].uuid());
     Printf("0x%zx-0x%zx %s (%s) %s\n", modules[i].base_address(),
            modules[i].max_executable_address(), modules[i].full_name(),
            ModuleArchToString(modules[i].arch()), uuid_str);
   }
   Printf("End of module map.\n");
 }
 
+void CheckNoDeepBind(const char *filename, int flag) {
+  // Do nothing.
+}
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_MAC

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

bool IsProcessRunning(pid_t pid) {
   return false;
 }
 
 int WaitForProcess(pid_t pid) { return -1; }
 
 // FIXME implement on this platform.
 void GetMemoryProfile(fill_profile_f cb, uptr *stats, uptr stats_size) { }
 
+void CheckNoDeepBind(const char *filename, int flag) {
+  // Do nothing.
+}
 
 }  // namespace __sanitizer
 
 #endif  // _WIN32

compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/deepbind.cc

+// RUN: %clangxx %s -o %t && %run not %t 1 2>&1 | FileCheck %s
+// UNSUPPORTED: lsan, android
+
+#include <dlfcn.h>
+#include <stdio.h>
+#include <string>
+
+int main (int argc, char *argv[]) {
+  // CHECK: You are trying to dlopen a <arbitrary path> shared library with RTLD_DEEPBIND flag
+  void *lib = dlopen("<arbitrary path>", RTLD_NOW | RTLD_DEEPBIND);
+  return 0;
+}