Redefine the ASan options for nacl_helper so that ASan does not handle SIGSEGV/SIGBUS

chrome/nacl/nacl_helper_linux.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // A mini-zygote specifically for Native Client.
 
 #include "chrome/common/nacl_helper_linux.h"
 
 #include <errno.h>
 #include <link.h>
@@ skipping 180 matching lines @@
     char *endp;
     prereserved_sandbox_size =
         strtoul(reserved_at_zero_switch_value.c_str(), &endp, 0);
     if (*endp != '\0')
       LOG(ERROR) << "Could not parse reserved_at_zero argument value of "
                  << reserved_at_zero_switch_value;
   }
   return prereserved_sandbox_size;
 }
 
+#if defined(ADDRESS_SANITIZER)

[Brad Chen (chromium), 2012/07/26 20:04:58]

I don't understand how this will change the behavior of the ASAN version of
Chrome. Apart from making the PQ test pass, does it have any further impact in
terms of feature/function? Am I reading this code correctly that it will change
behavior of ASAN but not of NaCl? 

[Alexander Potapenko, 2012/07/27 08:42:54]

This will prevent ASan from installing its signal handler, which shall allow
NaCl to install that of his own. This means that every crash that happens e.g.
because of an access violation (but NOT an OOB read or other bug caught by ASan)
will result in executing NaCl SEGV handler.
Now can you please tell whether this is fine or not to use your SEGV handler?
Will it report a crash message? Will it send a crash to the crash server? Is it
more secure than the handler installed by ASan (I'm assuming yes)?

[Mark Seaborn, 2012/07/27 17:21:03]

It has two other effects:

 1) It will allow NaCl's fault handling to work under Asan (untrusted exception
handling, debug stub, and in the future, Breakpad).
 2) If Asan catches a memory error in NaCl on Linux, we won't get a helpful
message for the location of the error (AFAIK).

We could fix (2) as I commented in
https://chromiumcodereview.appspot.com/10095012/.  BTW, Alexander, I was not
really sure why you dropped that change.

+// Do not install the SIGSEGV handler in ASan. This should make the NaCl
+// platform qualification test pass.
+const char *kAsanDefaultOptionsNaCl = "handle_segv=0";

[Mark Seaborn, 2012/07/27 17:12:02]

The Chromium style is
  const char kConstant[] = "blah";
like the constants earlier in the file.

Also, this can be static, right?

[Alexander Potapenko, 2012/07/27 18:51:23]

Done.

+
+// Override the default ASan options for the NaCl helper.
+// __asan_default_options should not be instrumented, because it is called
+// before ASan is initialized.
+extern "C"
+__attribute__((no_address_safety_analysis))
+const char *__asan_default_options() {
+  return kAsanDefaultOptionsNaCl;
+}
+#endif
+
 int main(int argc, char *argv[]) {
   CommandLine::Init(argc, argv);
   base::AtExitManager exit_manager;
   base::RandUint64();  // acquire /dev/urandom fd before sandbox is raised
 #if !defined(CHROMIUM_SELINUX)
   // Allows NSS to fopen() /dev/urandom.
   sandbox::InitLibcUrandomOverrides();
 #endif
 #if defined(USE_NSS)
   // Configure NSS for use inside the NaCl process.
@@ skipping 48 matching lines @@
       _exit(-1);
     }
     // if fork fails, send PID=-1 to zygote
     if (!UnixDomainSocket::SendMsg(kNaClZygoteDescriptor, &badpid,
                                    sizeof(badpid), empty)) {
       LOG(ERROR) << "*** send() to zygote failed";
     }
   }
   CHECK(false);  // This routine must not return
 }