aboutsummaryrefslogtreecommitdiff
path: root/src/client/linux/handler
diff options
context:
space:
mode:
authorthestig@chromium.org <thestig@chromium.org>2014-09-23 20:30:09 +0000
committerthestig@chromium.org <thestig@chromium.org>2014-09-23 20:30:09 +0000
commit37a3b8d997aa22da0f7ec741d443f332f58b3eda (patch)
treef768ce428e4166129af3983a93bfef08388c68a1 /src/client/linux/handler
parentFix clang compilation error introduced in r1380. (diff)
downloadbreakpad-37a3b8d997aa22da0f7ec741d443f332f58b3eda.tar.xz
Linux: Call memset() in a couple places in ExceptionHandler to avoid uninit memory reads under Valgrind.
Also move private static variables into the .cc file. BUG=chromium:332335 R=ivanpe@chromium.org Review URL: https://breakpad.appspot.com/5734002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1385 4c0a9323-5329-0410-9bdc-e9ce6186880e
Diffstat (limited to 'src/client/linux/handler')
-rw-r--r--src/client/linux/handler/exception_handler.cc52
-rw-r--r--src/client/linux/handler/exception_handler.h15
2 files changed, 32 insertions, 35 deletions
diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc
index f30e4594..db2f1e62 100644
--- a/src/client/linux/handler/exception_handler.cc
+++ b/src/client/linux/handler/exception_handler.cc
@@ -68,6 +68,7 @@
#include <errno.h>
#include <fcntl.h>
#include <linux/limits.h>
+#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdio.h>
@@ -150,6 +151,7 @@ void InstallAlternateStackLocked() {
old_stack.ss_size < kSigStackSize) {
new_stack.ss_sp = malloc(kSigStackSize);
new_stack.ss_size = kSigStackSize;
+ memset(new_stack.ss_sp, 0, kSigStackSize);
if (sys_sigaltstack(&new_stack, NULL) == -1) {
free(new_stack.ss_sp);
@@ -186,13 +188,13 @@ void RestoreAlternateStackLocked() {
stack_installed = false;
}
-} // namespace
+// The global exception handler stack. This is need 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;
-// We can stack multiple exception handlers. In that case, this is the global
-// which holds the stack.
-std::vector<ExceptionHandler*>* ExceptionHandler::handler_stack_ = NULL;
-pthread_mutex_t ExceptionHandler::handler_stack_mutex_ =
- PTHREAD_MUTEX_INITIALIZER;
+} // namespace
// Runs before crashing: normal context.
ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor,
@@ -212,30 +214,30 @@ ExceptionHandler::ExceptionHandler(const MinidumpDescriptor& descriptor,
if (!IsOutOfProcess() && !minidump_descriptor_.IsFD())
minidump_descriptor_.UpdatePath();
- pthread_mutex_lock(&handler_stack_mutex_);
- if (!handler_stack_)
- handler_stack_ = new std::vector<ExceptionHandler*>;
+ pthread_mutex_lock(&g_handler_stack_mutex_);
+ if (!g_handler_stack_)
+ g_handler_stack_ = new std::vector<ExceptionHandler*>;
if (install_handler) {
InstallAlternateStackLocked();
InstallHandlersLocked();
}
- handler_stack_->push_back(this);
- pthread_mutex_unlock(&handler_stack_mutex_);
+ g_handler_stack_->push_back(this);
+ pthread_mutex_unlock(&g_handler_stack_mutex_);
}
// Runs before crashing: normal context.
ExceptionHandler::~ExceptionHandler() {
- pthread_mutex_lock(&handler_stack_mutex_);
+ pthread_mutex_lock(&g_handler_stack_mutex_);
std::vector<ExceptionHandler*>::iterator handler =
- std::find(handler_stack_->begin(), handler_stack_->end(), this);
- handler_stack_->erase(handler);
- if (handler_stack_->empty()) {
- delete handler_stack_;
- handler_stack_ = NULL;
+ std::find(g_handler_stack_->begin(), g_handler_stack_->end(), this);
+ g_handler_stack_->erase(handler);
+ if (g_handler_stack_->empty()) {
+ delete g_handler_stack_;
+ g_handler_stack_ = NULL;
RestoreAlternateStackLocked();
RestoreHandlersLocked();
}
- pthread_mutex_unlock(&handler_stack_mutex_);
+ pthread_mutex_unlock(&g_handler_stack_mutex_);
}
// Runs before crashing: normal context.
@@ -295,7 +297,7 @@ void ExceptionHandler::RestoreHandlersLocked() {
// static
void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
// All the exception signals are blocked at this point.
- pthread_mutex_lock(&handler_stack_mutex_);
+ pthread_mutex_lock(&g_handler_stack_mutex_);
// Sometimes, Breakpad runs inside a process where some other buggy code
// saves and restores signal handlers temporarily with 'signal'
@@ -322,13 +324,13 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
// default one to avoid an infinite loop here.
signal(sig, SIG_DFL);
}
- pthread_mutex_unlock(&handler_stack_mutex_);
+ pthread_mutex_unlock(&g_handler_stack_mutex_);
return;
}
bool handled = false;
- for (int i = handler_stack_->size() - 1; !handled && i >= 0; --i) {
- handled = (*handler_stack_)[i]->HandleSignal(sig, info, uc);
+ for (int i = g_handler_stack_->size() - 1; !handled && i >= 0; --i) {
+ handled = (*g_handler_stack_)[i]->HandleSignal(sig, info, uc);
}
// Upon returning from this signal handler, sig will become unmasked and then
@@ -342,7 +344,7 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
RestoreHandlersLocked();
}
- pthread_mutex_unlock(&handler_stack_mutex_);
+ pthread_mutex_unlock(&g_handler_stack_mutex_);
if (info->si_pid || sig == SIGABRT) {
// This signal was triggered by somebody sending us the signal with kill().
@@ -398,6 +400,8 @@ bool ExceptionHandler::HandleSignal(int sig, siginfo_t* info, void* uc) {
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));
#if defined(__aarch64__)
@@ -449,7 +453,7 @@ bool ExceptionHandler::GenerateDump(CrashContext *context) {
// of caution than smash it into random locations.
static const unsigned kChildStackSize = 16000;
PageAllocator allocator;
- uint8_t* stack = (uint8_t*) allocator.Alloc(kChildStackSize);
+ uint8_t* stack = reinterpret_cast<uint8_t*>(allocator.Alloc(kChildStackSize));
if (!stack)
return false;
// clone() needs the top-most address. (scrub just to be safe)
diff --git a/src/client/linux/handler/exception_handler.h b/src/client/linux/handler/exception_handler.h
index bb88b950..591c3108 100644
--- a/src/client/linux/handler/exception_handler.h
+++ b/src/client/linux/handler/exception_handler.h
@@ -30,15 +30,13 @@
#ifndef CLIENT_LINUX_HANDLER_EXCEPTION_HANDLER_H_
#define CLIENT_LINUX_HANDLER_EXCEPTION_HANDLER_H_
-#include <string>
-#include <vector>
-
-#include <pthread.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/ucontext.h>
+#include <string>
+
#include "client/linux/crash_generation/crash_generation_client.h"
#include "client/linux/handler/minidump_descriptor.h"
#include "client/linux/minidump_writer/minidump_writer.h"
@@ -129,7 +127,7 @@ class ExceptionHandler {
ExceptionHandler(const MinidumpDescriptor& descriptor,
FilterCallback filter,
MinidumpCallback callback,
- void *callback_context,
+ void* callback_context,
bool install_handler,
const int server_fd);
~ExceptionHandler();
@@ -228,6 +226,7 @@ class ExceptionHandler {
// Report a crash signal from an SA_SIGINFO signal handler.
bool HandleSignal(int sig, siginfo_t* info, void* uc);
+
private:
// Save the old signal handlers and install new ones.
static bool InstallHandlersLocked();
@@ -258,12 +257,6 @@ class ExceptionHandler {
// believes are never read.
volatile HandlerCallback crash_handler_;
- // The global exception handler stack. This is need becuase there may exist
- // multiple ExceptionHandler instances in a process. Each will have itself
- // registered in this stack.
- static std::vector<ExceptionHandler*> *handler_stack_;
- static pthread_mutex_t handler_stack_mutex_;
-
// We need to explicitly enable ptrace of parent processes on some
// kernels, but we need to know the PID of the cloned process before we
// can do this. We create a pipe which we can use to block the