From f72b9c6ff4b6c48d4a2239c7da31e5dbf220c440 Mon Sep 17 00:00:00 2001 From: "digit@chromium.org" Date: Tue, 9 Oct 2012 18:03:25 +0000 Subject: 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 --- .../linux/handler/exception_handler_unittest.cc | 31 ++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'src/client/linux/handler/exception_handler_unittest.cc') diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index 890061e6..bb463510 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -317,6 +317,37 @@ TEST(ExceptionHandlerTest, RedeliveryToDefaultHandler) { 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(NULL) = 0; + } + // SIGKILL means Breakpad's signal handler didn't crash. + ASSERT_NO_FATAL_FAILURE(WaitForProcessToTerminate(child, SIGKILL)); +} + TEST(ExceptionHandlerTest, StackedHandlersDeliveredToTop) { AutoTempDir temp_dir; -- cgit v1.2.1