[compiler-rt] Make MaybeReexec properly process DYLD_INSERT_LIBRARIES when using non-absolute paths

[kubamracek]

Summary

MaybeReexec() in asan_mac.cc checks for presence of the ASan dylib in DYLD_INSERT_LIBRARIES, and if it is there, it will process this env. var. and remove the dylib from its value, so that spawned children don't have this variable set.

However, the current implementation only works when using a canonical absolute path to the dylib, it fails to remove the dylib for example when using @executable_path:

$ cat a.c
int main() {
    printf("Hello world.\n");
    system("ls");
}
$ ls
a.c
libclang_rt.asan_osx_dynamic.dylib
$ clang -fsanitize=address a.c
$ ./a.out
a.c
a.out
libclang_rt.asan_osx_dynamic.dylib
$ DYLD_INSERT_LIBRARIES=@executable_path/libclang_rt.asan_osx_dynamic.dylib ./a.out
Hello world.
dyld: could not load inserted library '@executable_path/libclang_rt.asan_osx_dynamic.dylib' because image not found
$ 

(here it's ls that fails to launch, because it's @executable_path is different)

This patch changes the processing of DYLD_INSERT_LIBRARIES to comparing values only based on filenames (ignoring directories).

[samsonov]

lib/sanitizer_common/sanitizer_libc.cc

33

You can write it as follows:

const char *t = (const char *)s;
void *res = nullptr;
for (uptr i = 0; i < n; ++i, ++t) {
  if (*t == c) res = reinterpret_cast<void*>(...);
}
return res;

[kubamracek]

Addressing review comments.

[kubamracek]

So it this okay to commit?

Thanks,
Kuba

[glider]

I'm ok with this patch.

[kubamracek]

Landed in r228392.

lib/asan/asan_mac.cc

   // ourselves.
   Dl_info info;
   CHECK(dladdr((void*)((uptr)__asan_init), &info));
   char *dyld_insert_libraries =
       const_cast<char*>(GetEnv(kDyldInsertLibraries));
   uptr old_env_len = dyld_insert_libraries ?
       internal_strlen(dyld_insert_libraries) : 0;
   uptr fname_len = internal_strlen(info.dli_fname);
+  const char *dylib_name = StripModuleName(info.dli_fname);
+  uptr dylib_name_len = internal_strlen(dylib_name);
   if (!dyld_insert_libraries ||
-      !REAL(strstr)(dyld_insert_libraries, StripModuleName(info.dli_fname))) {
+      !REAL(strstr)(dyld_insert_libraries, dylib_name)) {
     // DYLD_INSERT_LIBRARIES is not set or does not contain the runtime
     // library.
     char program_name[1024];
     uint32_t buf_size = sizeof(program_name);
     _NSGetExecutablePath(program_name, &buf_size);
     char *new_env = const_cast<char*>(info.dli_fname);
     if (dyld_insert_libraries) {
       // Append the runtime dylib name to the existing value of

 
     // We get here only if execv() failed.
     Report("ERROR: The process is launched without DYLD_INSERT_LIBRARIES, "
            "which is required for ASan to work. ASan tried to set the "
            "environment variable and re-execute itself, but execv() failed, "
            "possibly because of sandbox restrictions. Make sure to launch the "
            "executable with:\n%s=%s\n", kDyldInsertLibraries, new_env);
     CHECK("execv failed" && 0);
-  } else {
-    // DYLD_INSERT_LIBRARIES is set and contains the runtime library.
-    if (old_env_len == fname_len) {
-      // It's just the runtime library name - fine to unset the variable.
-      LeakyResetEnv(kDyldInsertLibraries, NULL);
+  }
+
+  // DYLD_INSERT_LIBRARIES is set and contains the runtime library. Let's remove
+  // the dylib from the environment variable, because interceptors are installed
+  // and we don't want our children to inherit the variable.
+
+  uptr env_name_len = internal_strlen(kDyldInsertLibraries);
+  // Allocate memory to hold the previous env var name, its value, the '='
+  // sign and the '\0' char.
+  char *new_env = (char*)allocator_for_env.Allocate(
+      old_env_len + 2 + env_name_len);
+  CHECK(new_env);
+  internal_memset(new_env, '\0', old_env_len + 2 + env_name_len);
+  internal_strncpy(new_env, kDyldInsertLibraries, env_name_len);
+  new_env[env_name_len] = '=';
+  char *new_env_pos = new_env + env_name_len + 1;
+
+  // Iterate over colon-separated pieces of |dyld_insert_libraries|.
+  char *piece_start = dyld_insert_libraries;
+  char *piece_end = NULL;
+  char *old_env_end = dyld_insert_libraries + old_env_len;
+  do {
+    if (piece_start[0] == ':') piece_start++;
+    piece_end = REAL(strchr)(piece_start, ':');
+    if (!piece_end) piece_end = dyld_insert_libraries + old_env_len;
+    if ((uptr)(piece_start - dyld_insert_libraries) > old_env_len) break;
+    uptr piece_len = piece_end - piece_start;
+
+    char *filename_start =
+        (char *)internal_memrchr(piece_start, '/', piece_len);
+    uptr filename_len = piece_len;
+    if (filename_start) {
+      filename_start += 1;
+      filename_len = piece_len - (filename_start - piece_start);
     } else {
-      uptr env_name_len = internal_strlen(kDyldInsertLibraries);
-      // Allocate memory to hold the previous env var name, its value, the '='
-      // sign and the '\0' char.
-      char *new_env = (char*)allocator_for_env.Allocate(
-          old_env_len + 2 + env_name_len);
-      CHECK(new_env);
-      internal_memset(new_env, '\0', old_env_len + 2 + env_name_len);
-      internal_strncpy(new_env, kDyldInsertLibraries, env_name_len);
-      new_env[env_name_len] = '=';
-      char *new_env_pos = new_env + env_name_len + 1;
-
-      // Iterate over colon-separated pieces of |dyld_insert_libraries|.
-      char *piece_start = dyld_insert_libraries;
-      char *piece_end = NULL;
-      char *old_env_end = dyld_insert_libraries + old_env_len;
-      do {
-        if (piece_start[0] == ':') piece_start++;
-        piece_end =  REAL(strchr)(piece_start, ':');
-        if (!piece_end) piece_end = dyld_insert_libraries + old_env_len;
-        if ((uptr)(piece_start - dyld_insert_libraries) > old_env_len) break;
-        uptr piece_len = piece_end - piece_start;
-
-        // If the current piece isn't the runtime library name,
-        // append it to new_env.
-        if ((piece_len != fname_len) ||
-            (internal_strncmp(piece_start, info.dli_fname, fname_len) != 0)) {
-          if (new_env_pos != new_env + env_name_len + 1) {
-            new_env_pos[0] = ':';
-            new_env_pos++;
-          }
-          internal_strncpy(new_env_pos, piece_start, piece_len);
-        }
-        // Move on to the next piece.
-        new_env_pos += piece_len;
-        piece_start = piece_end;
-      } while (piece_start < old_env_end);
-
-      // Can't use setenv() here, because it requires the allocator to be
-      // initialized.
-      // FIXME: instead of filtering DYLD_INSERT_LIBRARIES here, do it in
-      // a separate function called after InitializeAllocator().
-      LeakyResetEnv(kDyldInsertLibraries, new_env);
+      filename_start = piece_start;
     }
-  }
+
+    // If the current piece isn't the runtime library name,
+    // append it to new_env.
+    if ((dylib_name_len != filename_len) ||
+        (internal_memcmp(filename_start, dylib_name, dylib_name_len) != 0)) {
+      if (new_env_pos != new_env + env_name_len + 1) {
+        new_env_pos[0] = ':';
+        new_env_pos++;
+      }
+      internal_strncpy(new_env_pos, piece_start, piece_len);
+      new_env_pos += piece_len;
+    }
+    // Move on to the next piece.
+    piece_start = piece_end;
+  } while (piece_start < old_env_end);
+
+  // Can't use setenv() here, because it requires the allocator to be
+  // initialized.
+  // FIXME: instead of filtering DYLD_INSERT_LIBRARIES here, do it in
+  // a separate function called after InitializeAllocator().
+  if (new_env_pos == new_env + env_name_len + 1) new_env = NULL;
+  LeakyResetEnv(kDyldInsertLibraries, new_env);
 }
 
 // No-op. Mac does not support static linkage anyway.
 void *AsanDoesNotSupportStaticLinkage() {
   return 0;
 }
 
 // No-op. Mac does not support static linkage anyway.

lib/sanitizer_common/sanitizer_libc.h

 
 namespace __sanitizer {
 
 // internal_X() is a custom implementation of X() for use in RTL.
 
 // String functions
 s64 internal_atoll(const char *nptr);
 void *internal_memchr(const void *s, int c, uptr n);
+void *internal_memrchr(const void *s, int c, uptr n);
 int internal_memcmp(const void* s1, const void* s2, uptr n);
 void *internal_memcpy(void *dest, const void *src, uptr n);
 void *internal_memmove(void *dest, const void *src, uptr n);
 // Set [s, s + n) to 0. Both s and n should be 16-aligned.
 void internal_bzero_aligned16(void *s, uptr n);
 // Should not be used in performance-critical places.
 void *internal_memset(void *s, int c, uptr n);
 char* internal_strchr(const char *s, int c);

lib/sanitizer_common/sanitizer_libc.cc

 void *internal_memchr(const void *s, int c, uptr n) {
   const char *t = (const char *)s;
   for (uptr i = 0; i < n; ++i, ++t)
     if (*t == c)
       return reinterpret_cast<void *>(const_cast<char *>(t));
   return 0;
 }
 
+void *internal_memrchr(const void *s, int c, uptr n) {
+  const char *t = (const char *)s;
+  void *res = nullptr;
+  for (uptr i = 0; i < n; ++i, ++t) {
+    if (*t == c) res = reinterpret_cast<void *>(const_cast<char *>(t));
+  }
+  return res;
+}
+
 int internal_memcmp(const void* s1, const void* s2, uptr n) {
   const char *t1 = (const char *)s1;
   const char *t2 = (const char *)s2;
   for (uptr i = 0; i < n; ++i, ++t1, ++t2)
     if (*t1 != *t2)
       return *t1 < *t2 ? -1 : 1;
   return 0;
 }

test/asan/TestCases/Darwin/dyld_insert_libraries_remove.cc

+// Check that when launching with DYLD_INSERT_LIBRARIES, we properly remove
+// the ASan dylib from the environment variable (both when using an absolute
+// or relative path) and also that the other dylibs are left untouched.
+
+// RUN: mkdir -p %T/dyld_insert_libraries_remove
+// RUN: cp `%clang_asan %s -fsanitize=address -### 2>&1 \
+// RUN:   | grep "libclang_rt.asan_osx_dynamic.dylib" \
+// RUN:   | sed -e 's/.*"\(.*libclang_rt.asan_osx_dynamic.dylib\)".*/\1/'` \
+// RUN:   %T/dyld_insert_libraries_remove/libclang_rt.asan_osx_dynamic.dylib
+
+// RUN: %clangxx_asan %s -o %T/dyld_insert_libraries_remove/a.out
+// RUN: %clangxx -DSHARED_LIB %s \
+// RUN:     -dynamiclib -o %T/dyld_insert_libraries_remove/dummy-so.dylib
+
+// RUN: ( cd %T/dyld_insert_libraries_remove && \
+// RUN:   DYLD_INSERT_LIBRARIES=@executable_path/libclang_rt.asan_osx_dynamic.dylib:dummy-so.dylib \
+// RUN:   %run ./a.out 2>&1 ) | FileCheck %s || exit 1
+
+// RUN: ( cd %T/dyld_insert_libraries_remove && \
+// RUN:   DYLD_INSERT_LIBRARIES=libclang_rt.asan_osx_dynamic.dylib:dummy-so.dylib \
+// RUN:   %run ./a.out 2>&1 ) | FileCheck %s || exit 1
+
+// RUN: ( cd %T/dyld_insert_libraries_remove && \
+// RUN:   DYLD_INSERT_LIBRARIES=%T/dyld_insert_libraries_remove/libclang_rt.asan_osx_dynamic.dylib:dummy-so.dylib \
+// RUN:   %run ./a.out 2>&1 ) | FileCheck %s || exit 1
+
+#if !defined(SHARED_LIB)
+#include <stdio.h>
+#include <stdlib.h>
+
+int main() {
+  const char kEnvName[] = "DYLD_INSERT_LIBRARIES";
+  printf("%s=%s\n", kEnvName, getenv(kEnvName));
+  // CHECK: {{DYLD_INSERT_LIBRARIES=dummy-so.dylib}}
+  return 0;
+}
+#else  // SHARED_LIB
+void foo() {}
+#endif  // SHARED_LIB