Make Linux signal handler more robust.

Breakpad can be used on processes where a mistaken
library saves then restores one of our signal handlers
with 'signal' instead of 'sigaction'.

This loses the SA_SIGINFO flag associated with the 
Breakpad handler, and in some cases (e.g. Android/ARM
kernels), the values of the 'info' and 'uc' parameters
that ExceptionHandler::SignalHandler() receives will
be completely bogus, leading to a crash when the function
is executed (and of course, no minidump generation).

To work-around this, have SignalHandler() check the state
of the flag. If it is incorrectly unset, re-register with
'sigaction' and the correct flag, then return. The signal
will be re-thrown, and this time the function will be
called with the correct values.
Review URL: https://breakpad.appspot.com/481002

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1067 4c0a9323-5329-0410-9bdc-e9ce6186880e
This commit is contained in:
digit@chromium.org 2012-10-09 18:03:25 +00:00
parent b52be69e59
commit f72b9c6ff4
2 changed files with 60 additions and 0 deletions

View file

@ -292,6 +292,35 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
// All the exception signals are blocked at this point. // All the exception signals are blocked at this point.
pthread_mutex_lock(&handler_stack_mutex_); pthread_mutex_lock(&handler_stack_mutex_);
// Sometimes, Breakpad runs inside a process where some other buggy code
// saves and restores signal handlers temporarily with 'signal'
// instead of 'sigaction'. This loses the SA_SIGINFO flag associated
// with this function. As a consequence, the values of 'info' and 'uc'
// become totally bogus, generally inducing a crash.
//
// The following code tries to detect this case. When it does, it
// resets the signal handlers with sigaction + SA_SIGINFO and returns.
// This forces the signal to be thrown again, but this time the kernel
// will call the function with the right arguments.
struct sigaction cur_handler;
if (sigaction(sig, NULL, &cur_handler) == 0 &&
(cur_handler.sa_flags & SA_SIGINFO) == 0) {
// Reset signal handler with the right flags.
sigemptyset(&cur_handler.sa_mask);
sigaddset(&cur_handler.sa_mask, sig);
cur_handler.sa_sigaction = SignalHandler;
cur_handler.sa_flags = SA_ONSTACK | SA_SIGINFO;
if (sigaction(sig, &cur_handler, NULL) == -1) {
// When resetting the handler fails, try to reset the
// default one to avoid an infinite loop here.
signal(sig, SIG_DFL);
}
pthread_mutex_unlock(&handler_stack_mutex_);
return;
}
bool handled = false; bool handled = false;
for (int i = handler_stack_->size() - 1; !handled && i >= 0; --i) { for (int i = handler_stack_->size() - 1; !handled && i >= 0; --i) {
handled = (*handler_stack_)[i]->HandleSignal(sig, info, uc); handled = (*handler_stack_)[i]->HandleSignal(sig, info, uc);

View file

@ -317,6 +317,37 @@ TEST(ExceptionHandlerTest, RedeliveryToDefaultHandler) {
ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGSEGV)); ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGSEGV));
} }
// Check that saving and restoring the signal handler with 'signal'
// instead of 'sigaction' doesn't make the Breakpad signal handler
// crash. See comments in ExceptionHandler::SignalHandler for full
// details.
TEST(ExceptionHandlerTest, RedeliveryOnBadSignalHandlerFlag) {
AutoTempDir temp_dir;
const pid_t child = fork();
if (child == 0) {
// Install the RaiseSIGKILL handler for SIGSEGV.
ASSERT_TRUE(InstallRaiseSIGKILL());
// Create a new exception handler, this installs a new SIGSEGV
// handler, after saving the old one.
ExceptionHandler handler(
MinidumpDescriptor(temp_dir.path()), NULL,
DoneCallbackReturnFalse, NULL, true, -1);
// Install the default SIGSEGV handler, saving the current one.
// Then re-install the current one with 'signal', this loses the
// SA_SIGINFO flag associated with the Breakpad handler.
sighandler_t old_handler = signal(SIGSEGV, SIG_DFL);
ASSERT_NE(old_handler, SIG_ERR);
ASSERT_NE(signal(SIGSEGV, old_handler), SIG_ERR);
// Crash with the exception handler in scope.
*reinterpret_cast<volatile int*>(NULL) = 0;
}
// SIGKILL means Breakpad's signal handler didn't crash.
ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGKILL));
}
TEST(ExceptionHandlerTest, StackedHandlersDeliveredToTop) { TEST(ExceptionHandlerTest, StackedHandlersDeliveredToTop) {
AutoTempDir temp_dir; AutoTempDir temp_dir;