Linux ExceptionHandler: don't allocate the CrashContext on the stack

src/client/linux/handler/exception_handler.cc

 // Copyright (c) 2010 Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 194 matching lines @@
   signal(sig, SIG_DFL);
 #endif
 }
 
 // The global exception handler stack. This is needed because there may exist
 // multiple ExceptionHandler instances in a process. Each will have itself
 // registered in this stack.
 std::vector<ExceptionHandler*>* g_handler_stack_ = NULL;
 pthread_mutex_t g_handler_stack_mutex_ = PTHREAD_MUTEX_INITIALIZER;
 
+// sizeof(CrashContext) can be too big w.r.t the size of alternatate stack
+// for SignalHandler(). Keep the crash context as a .bss field. Exception
+// handlers are serialized by the |g_handler_stack_mutex_| and at most one at a
+// time can use |g_crash_context_|.
+ExceptionHandler::CrashContext g_crash_context_;
+
 }  // namespace
 
 // Runs before crashing: normal context.
 ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor,
                                    FilterCallback filter,
                                    MinidumpCallback callback,
                                    void* callback_context,
                                    bool install_handler,
                                    const int server_fd)
     : filter_(filter),
       callback_(callback),
       callback_context_(callback_context),
       minidump_descriptor_(descriptor),
       crash_handler_(NULL) {
   if (server_fd >= 0)
     crash_generation_client_.reset(CrashGenerationClient::TryCreate(server_fd));
 
   if (!IsOutOfProcess() && !minidump_descriptor_.IsFD() &&
       !minidump_descriptor_.IsMicrodumpOnConsole())
     minidump_descriptor_.UpdatePath();
 
 #if defined(__ANDROID__)
   if (minidump_descriptor_.IsMicrodumpOnConsole())
     logger::initializeCrashLogWriter();
 #endif
 
   pthread_mutex_lock(&g_handler_stack_mutex_);
+
+  // Pre-fault the crash context struct. This is to avoid failing due to OOM

[Mark Mentovai, 2015/09/18 22:13:20]

Smart.

+  // if handling an exception when the process ran out of virtual memory.
+  memset(&g_crash_context_, 0, sizeof(g_crash_context_));
+
   if (!g_handler_stack_)
     g_handler_stack_ = new std::vector<ExceptionHandler*>;
   if (install_handler) {
     InstallAlternateStackLocked();
     InstallHandlersLocked();
   }
   g_handler_stack_->push_back(this);
   pthread_mutex_unlock(&g_handler_stack_mutex_);
 }
 
@@ skipping 165 matching lines @@
   if (filter_ && !filter_(callback_context_))
     return false;
 
   // Allow ourselves to be dumped if the signal is trusted.
   bool signal_trusted = info->si_code > 0;
   bool signal_pid_trusted = info->si_code == SI_USER ||
       info->si_code == SI_TKILL;
   if (signal_trusted || (signal_pid_trusted && info->si_pid == getpid())) {
     sys_prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
   }
-  CrashContext context;
+
   // Fill in all the holes in the struct to make Valgrind happy.
-  memset(&context, 0, sizeof(context));
-  memcpy(&context.siginfo, info, sizeof(siginfo_t));
-  memcpy(&context.context, uc, sizeof(struct ucontext));
+  memset(&g_crash_context_, 0, sizeof(g_crash_context_));

[Mark Mentovai, 2015/09/18 22:13:20]

Necessary? You already wrote zeroes into it when you created the
ExceptionHandler now.

[Primiano Tucci (use gerrit), 2015/09/18 22:22:40]

Right, should be safe from the production code anyways, just not sure about the
Asan/valgrind behavior.
Should we remove this?
To be honest, the siginfo and .context below seem absolutely redundant to me, as
they are part of the context struct.
I'd just remove all three at this point. 

The only question might be: is it ever the case that breakpad can handle more
than one signal?
IIRC it should self-unregister after handling the firrst signal.
If there is a case where we want to handle more than one signal per process, the
memset above should stay.

[Mark Mentovai, 2015/09/18 22:34:56]

Primiano Tucci wrote:
I think that they shouldn’t care as long as it was touched once.
That’s fair, let’s leave it for safety. I think we normally will handle only
one, but I don’t have the inclination to check whether simulated crashes (that
shouldn’t terminate the process) are also routed through HandleSignal.

+  memcpy(&g_crash_context_.siginfo, info, sizeof(siginfo_t));
+  memcpy(&g_crash_context_.context, uc, sizeof(struct ucontext));
 #if defined(__aarch64__)
-  struct ucontext *uc_ptr = (struct ucontext*)uc;
-  struct fpsimd_context *fp_ptr =
+  struct ucontext* uc_ptr = (struct ucontext*)uc;
+  struct fpsimd_context* fp_ptr =
       (struct fpsimd_context*)&uc_ptr->uc_mcontext.__reserved;
   if (fp_ptr->head.magic == FPSIMD_MAGIC) {
-    memcpy(&context.float_state, fp_ptr, sizeof(context.float_state));
+    memcpy(&g_crash_context_.float_state, fp_ptr,
+           sizeof(g_crash_context_.float_state));
   }
-#elif !defined(__ARM_EABI__)  && !defined(__mips__)
+#elif !defined(__ARM_EABI__) && !defined(__mips__)
   // FP state is not part of user ABI on ARM Linux.
   // In case of MIPS Linux FP state is already part of struct ucontext
   // and 'float_state' is not a member of CrashContext.
-  struct ucontext *uc_ptr = (struct ucontext*)uc;
+  struct ucontext* uc_ptr = (struct ucontext*)uc;
   if (uc_ptr->uc_mcontext.fpregs) {
-    memcpy(&context.float_state,
-           uc_ptr->uc_mcontext.fpregs,
-           sizeof(context.float_state));
+    memcpy(&g_crash_context_.float_state, uc_ptr->uc_mcontext.fpregs,
+           sizeof(g_crash_context_.float_state));
   }
 #endif
-  context.tid = syscall(__NR_gettid);
+  g_crash_context_.tid = syscall(__NR_gettid);
   if (crash_handler_ != NULL) {
-    if (crash_handler_(&context, sizeof(context), callback_context_)) {
+    if (crash_handler_(&g_crash_context_, sizeof(g_crash_context_),
+                       callback_context_)) {
       return true;
     }
   }
-  return GenerateDump(&context);
+  return GenerateDump(&g_crash_context_);
 }
 
 // This is a public interface to HandleSignal that allows the client to
 // generate a crash dump. This function may run in a compromised context.
 bool ExceptionHandler::SimulateSignalDelivery(int sig) {
   siginfo_t siginfo = {};
   // Mimic a trusted signal to allow tracing the process (see
   // ExceptionHandler::HandleSignal().
   siginfo.si_code = SI_USER;
   siginfo.si_pid = getpid();
@@ skipping 278 matching lines @@
   descriptor.UpdatePath();
   if (!google_breakpad::WriteMinidump(descriptor.path(),
                                       child,
                                       child_blamed_thread))
       return false;
 
   return callback ? callback(descriptor, callback_context, true) : true;
 }
 
 }  // namespace google_breakpad