[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

Does it make sense to print backtrace?

test/sanitizer_common/TestCases/Posix/deepbind.cc

1

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

4

Android does not have it?

18

You can just return 0 after this.

[kubamracek]

lib/sanitizer_common/sanitizer_common.cc

480

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

[m.ostapenko]

lib/sanitizer_common/sanitizer_common.cc

480

Thanks, will move this to sanitizer_linux. cc.

485

Perhaps. At least we should print library name here.

test/sanitizer_common/TestCases/Posix/deepbind.cc

1

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

4

AFAIK Android doesn't intercept dlopen.

[m.ostapenko]

Updating.

[m.ostapenko]

lib/sanitizer_common/sanitizer_common.cc

485

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.

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(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();                                                   \

lib/sanitizer_common/sanitizer_common.cc

 #include "sanitizer_common.h"
 #include "sanitizer_allocator_interface.h"
 #include "sanitizer_allocator_internal.h"
 #include "sanitizer_flags.h"
 #include "sanitizer_libc.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_stacktrace_printer.h"
 #include "sanitizer_symbolizer.h"
+#include <dlfcn.h> // For RTLD_DEEPBIND
 
 namespace __sanitizer {
 
 const char *SanitizerToolName = "SanitizerTool";
 
 atomic_uint32_t current_verbosity;
 uptr PageSizeCached;
 
if (MFHooks[i].malloc_hook == nullptr) {
       MFHooks[i].malloc_hook = malloc_hook;
       MFHooks[i].free_hook = free_hook;
       return i + 1;
     }
   }
   return 0;
 }
 
+void CheckNoDeepBind(int flag) {
+  if (flag & RTLD_DEEPBIND) {
+    Report("You are trying to dlopen a 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 your library under sanitizers please remove "
+           "RTLD_DEEPBIND from dlopen flags.\n");
+    Die();
+  }
+}
+
 } // namespace __sanitizer
 
 using namespace __sanitizer;  // NOLINT
 
 extern "C" {
 void __sanitizer_set_report_path(const char *path) {
   report_file.SetReportPath(path);
 }

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(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) {}

test/sanitizer_common/TestCases/Posix/deepbind.cc

+// RUN: %clangxx -DSHARED_LIB %s -fPIC -shared -o %t-so.so
+// RUN: %clangxx %s -o %t && %run %t 2>&1
+// RUN: %run not %t 1 2>&1 | FileCheck %s --check-prefix CHECK-DEEPBIND
+// UNSUPPORTED: lsan, android
+
+#if !defined(SHARED_LIB)
+#include <dlfcn.h>
+#include <stdio.h>
+#include <string>
+
+using std::string;
+
+int main (int argc, char *argv[]) {
+  int flag = argc > 1 ? RTLD_NOW | RTLD_DEEPBIND : RTLD_NOW;
+  string path = string(argv[0]) + "-so.so";
+  printf("opening %s ... \n", path.c_str());
+  // CHECK-DEEPBIND: You are trying to dlopen a shared library with RTLD_DEEPBIND flag
+  void *lib = dlopen(path.c_str(), flag);
+  if (!lib) {
+    printf("error in dlopen(): %s\n", dlerror());
+    return 1;
+  }
+  dlclose(lib);
+  return 0;
+}
+#else  // SHARED_LIB
+#include <stdio.h>
+
+__attribute__((constructor))
+void at_dlopen() {
+  printf("%s: I am being dlopened\n", __FILE__);
+}
+#endif