From 4d06db5a1f5d3fb2f25dbc40577ca4e2149911b0 Mon Sep 17 00:00:00 2001 From: Primiano Tucci Date: Tue, 22 Sep 2015 09:11:24 +0100 Subject: [PATCH] Linux ExceptionHandler: don't allocate the CrashContext on the stack On Android the size of the alternate stack can be very small (8k). Even if breakpad uses sigaltstack to increase the size of the alternate stack during initialization, that call affects only the main thread. On Android, the libc's pthread initializer reset the sigaltstack to 8k. When entering a signal handler, the kernel typically pushes the context on the alternate stack. On arm64, sizeof(CrashContext) is ~5k, which leaves 3k of usable stack for breakpad. On top of that, breakpad allocates another struct CrashContext on the stack. In the case of Android arm64, then, breakpad ends up using 5k + 5k > 8k of stack, which causes a stack overflow. This got unnoticed in Android L, as the alternate stack didn't have red-zones between them, so breakpad was often happily overflowing onto the next thread's stack. This is not the case anymore [1]. This CL moves the CrashContext into a global variable. It should be safe as the ExceptionHandlers are serialized on a mutex. [1] https://android.googlesource.com/platform/bionic/+/595752f623ae88f7e4193a6e531a0805f1c6c4dc BUG=374 R=mark@chromium.org Review URL: https://codereview.chromium.org/1354923002 . --- src/client/linux/handler/exception_handler.cc | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc index 9b20fe25..5bd2d8cc 100644 --- a/src/client/linux/handler/exception_handler.cc +++ b/src/client/linux/handler/exception_handler.cc @@ -212,6 +212,12 @@ void InstallDefaultHandler(int sig) { std::vector* 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. @@ -239,6 +245,11 @@ ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor, #endif pthread_mutex_lock(&g_handler_stack_mutex_); + + // Pre-fault the crash context struct. This is to avoid failing due to OOM + // 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; if (install_handler) { @@ -424,36 +435,37 @@ bool ExceptionHandler::HandleSignal(int sig, siginfo_t* info, void* uc) { 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_)); + 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