Clear LD_PRELOAD when forking external symbolizer.

[samsonov]

Why not simply unsetenv("LD_PRELOAD")?

[glider]

lib/sanitizer_common/sanitizer_symbolizer_linux.cc

103

If LD_PRELOAD contains more than one library, you only need to remove yours, not unset LD_PRELOAD completely.

[earthdok]

Why not simply unsetenv("LD_PRELOAD")?

I thought unsetenv acquired a lock, which in fact it doesn't.

lib/sanitizer_common/sanitizer_symbolizer_linux.cc

103

How do we know which one is ours?

[glider]

lib/sanitizer_common/sanitizer_symbolizer_linux.cc

103

You can obtain the library name from dladdr().

[earthdok]

• removed custom unsetenv implementation
• added a fixme

lib/sanitizer_common/sanitizer_symbolizer_linux.cc

103

Adding a FIXME for now, as per our offline discussion.

[glider]

LGTM

[kcc]

lib/sanitizer_common/sanitizer_symbolizer_linux.cc

107

Will this work with bash?
(bash has its own setenv/getenv. Maybe unsetenv too. I'd prefer to have a custom implementation if possible)

[samsonov]

Can we do a "custom" implementation by dlsym("unsetenv") lookup (similar to
current SetEnv implementation)?

[earthdok]

• reverted to a custom implementation of unsetenv

Also, modified the implementation to allow multiple entries for the same
variable.

[earthdok]

I couldn't find anything related to setenv in bash sources, but I reverted to a custom implementation anyway.

Can we do a "custom" implementation by dlsym("unsetenv") lookup (similar to current SetEnv implementation)?

No. It's not safe to call dlsym after a fork.

lib/sanitizer_common/sanitizer_common.h

 void *MapFileToMemory(const char *file_name, uptr *buff_size);
 
 // OS
 void DisableCoreDumper();
 void DumpProcessMap();
 bool FileExists(const char *filename);
 const char *GetEnv(const char *name);
 bool SetEnv(const char *name, const char *value);
+void UnsetEnv(const char *name);
 const char *GetPwd();
 u32 GetUid();
 void ReExec();
 bool StackSizeIsUnlimited();
 void SetStackSizeLimitInBytes(uptr limit);
 void PrepareForSandboxing();
 
 void InitTlsSize();

lib/sanitizer_common/sanitizer_linux.cc

bool SetEnv(const char *name, const char *value) {
   typedef int(*setenv_ft)(const char *name, const char *value, int overwrite);
   setenv_ft setenv_f;
   CHECK_EQ(sizeof(setenv_f), sizeof(f));
   internal_memcpy(&setenv_f, &f, sizeof(f));
   return setenv_f(name, value, 1) == 0;
 }
 #endif
 
+void UnsetEnv(const char *name) {
+  uptr name_length = internal_strlen(name);
+  uptr last = 0;
+  while (environ[last]) last++;
+  for (uptr i = 0; environ[i]; i++)
+    if (internal_strlen(environ[i]) >= name_length + 1 &&
+        internal_strncmp(environ[i], name, name_length) == 0 &&
+        environ[i][name_length] == '=') {
+      last--;
+      environ[i] = environ[last];
+      environ[last] = 0;
+    }
+}
+
 #ifdef __GLIBC__
 
 extern "C" {
   extern void *__libc_stack_end;
 }
 
 static void GetArgsAndEnv(char ***argv, char ***envp) {
   uptr *stack_end = (uptr *)__libc_stack_end;

lib/sanitizer_common/sanitizer_symbolizer_linux.cc

if (pid == -1) {
     internal_dup2(outfd[0], STDIN_FILENO);
     internal_dup2(infd[1], STDOUT_FILENO);
     internal_close(outfd[0]);
     internal_close(outfd[1]);
     internal_close(infd[0]);
     internal_close(infd[1]);
     for (int fd = getdtablesize(); fd > 2; fd--)
       internal_close(fd);
+    // If the parent tool is used as a preloadable library, do not apply it to
+    // the symbolizer.
+    // FIXME: If LD_PRELOAD contains more than one object, we should keep those
+    // that have nothing to do with us.
+    UnsetEnv("LD_PRELOAD");
     execl(path_to_symbolizer, path_to_symbolizer, (char*)0);
     internal__exit(1);
   }
 
   // Continue execution in parent process.
   internal_close(outfd[0]);
   internal_close(infd[1]);
   *input_fd = infd[0];