5 Commits

Author SHA1 Message Date
primiano@chromium.org
c86860f5e2 Merge trunk r1454 to the chrome_43 branch.
> Fix signal propagation logic for Linux/Android exception handler.
>
> The current code is relying on info->si_pid to figure out whether
> the exception handler was triggered by a signal coming from the kernel
> (that will re-trigger until the cause that triggered the signal has
> been cleared) or from user-space e.g., kill -SIGNAL pid, which will NOT
> automatically re-trigger in the next signal handler in the chain.
> While the intentions are good (manually re-triggering user-space
> signals), the current implementation mistakenly looks at the si_pid
> field in siginfo_t, assuming that it is coming from the kernel if
> si_pid == 0.
> This is wrong. siginfo_t, in fact, is a union and si_pid is meaningful
> only for userspace signals. For signals originated by the kernel,
> instead, si_pid overlaps with si_addr (the faulting address).
> As a matter of facts, the current implementation is mistakenly
> re-triggering the signal using tgkill for most of the kernel-space
> signals (unless the fault address is exactly 0x0).
> This is not completelly correct for the case of SIGSEGV/SIGBUS. The
> next handler in the chain will stil see the signal, but the |siginfo|
> and the |context| arguments of the handler will be meaningless
> (retriggering a signal with tgkill doesn't preserve them).
> Therefore, if the next handler in the chain expects those arguments
> to be set, it will fail.
> Concretelly, this is causing problems to WebView. In some rare
> circumstances, the next handler in the chain is a user-space runtime
> which does SIGSEGV handling to implement speculative null pointer
> managed exceptions (see as an example
> http://www.mono-project.com/docs/advanced/runtime/docs/exception-handling/)
>
> The fix herein proposed consists in using the si_code (see SI_FROMUSER
> macros) to determine whether a signal is coming form the kernel
> (and therefore just re-establish the next signal handler) or from
> userspace (and use the tgkill logic).
>
> Repro case:
> This issue is visible in Chrome for Android with this simple repro case:
> - Add a non-null pointer dereference in the codebase:
>   *((volatile int*)0xbeef) = 42
> Without this change: the next handler (the libc trap) prints:
>   F/libc  (  595): Fatal signal 11 (SIGSEGV), code 1, fault addr 0x487
>   where 0x487 is actually the PID of the process (which is wrong).
> With this change: the next handler prints:
>   F/libc  (  595): Fatal signal 11 (SIGSEGV), code 1, fault addr 0xbeef
>   which is the correct answer.
>
> BUG=chromium:481937
> R=mark@chromium.org
>
> Review URL: https://breakpad.appspot.com/6844002

BUG=chromium:481937

git-svn-id: http://google-breakpad.googlecode.com/svn/branches/chrome_43@1457 4c0a9323-5329-0410-9bdc-e9ce6186880e
2015-05-19 08:01:12 +00:00
cjhopman@chromium.org
ef04c53283 Merge 1447 "Fix call to rt_sigaction"
> Fix call to rt_sigaction
> 
> Despite the fact that many places imply that sigaction and rt_sigaction
> are essentially the same, rt_sigaction's signature is actually
> different-- it takes the size of the kernel's sigset_t as an extra argument.
> 
> BUG=473973
> 

TBR=cjhopman@chromium.org

Review URL: https://breakpad.appspot.com/6834002

git-svn-id: http://google-breakpad.googlecode.com/svn/branches/chrome_43@1449 4c0a9323-5329-0410-9bdc-e9ce6186880e
2015-04-15 23:13:38 +00:00
mark@chromium.org
897c1bb4d6 Merge trunk r1443 to the chrome_43 branch.
Use __NR_rt_sigaction instead of __NR_sigaction

__NR_sigaction is not defined on arm64/x64/etc (or rather, it's defined
in unistd-32.h instead of unistd.h).

Patch by Chris Hopman <cjhopman@chromium.org>
Review URL: https://breakpad.appspot.com/10724002/


git-svn-id: http://google-breakpad.googlecode.com/svn/branches/chrome_43@1445 4c0a9323-5329-0410-9bdc-e9ce6186880e
2015-04-14 00:01:05 +00:00
mark@chromium.org
7ac616335d Merge trunk r1438 to the chrome_43 branch.
Workaround Android sigaction bug

On Android L+, signal and sigaction symbols are provided by libsigchain
that override the system's versions. There is a bug in these functions
where they essentially ignore requests to install SIG_DFL.

Workaround this issue by explicitly performing a syscall to
__NR_rt_sigaction to install SIG_DFL on Android.

BUG=473973

Patch by Chris Hopman <cjhopman@chromium.org>
Review URL: https://breakpad.appspot.com/1804002/


git-svn-id: http://google-breakpad.googlecode.com/svn/branches/chrome_43@1440 4c0a9323-5329-0410-9bdc-e9ce6186880e
2015-04-10 18:03:12 +00:00
mark@chromium.org
5b47d9d474 Branch at trunk r1434 for Chrome 43.0.2357
git-svn-id: http://google-breakpad.googlecode.com/svn/branches/chrome_43@1437 4c0a9323-5329-0410-9bdc-e9ce6186880e
2015-04-10 17:54:42 +00:00

View File

@@ -188,6 +188,24 @@ void RestoreAlternateStackLocked() {
stack_installed = false;
}
void InstallDefaultHandler(int sig) {
#if defined(__ANDROID__)
// Android L+ expose signal and sigaction symbols that override the system
// ones. There is a bug in these functions where a request to set the handler
// to SIG_DFL is ignored. In that case, an infinite loop is entered as the
// signal is repeatedly sent to breakpad's signal handler.
// To work around this, directly call the system's sigaction.
struct kernel_sigaction sa;
memset(&sa, 0, sizeof(sa));
sys_sigemptyset(&sa.sa_mask);
sa.sa_handler_ = SIG_DFL;
sa.sa_flags = SA_RESTART;
sys_rt_sigaction(sig, &sa, NULL, sizeof(kernel_sigset_t));
#else
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.
@@ -283,7 +301,7 @@ void ExceptionHandler::RestoreHandlersLocked() {
for (int i = 0; i < kNumHandledSignals; ++i) {
if (sigaction(kExceptionSignals[i], &old_handlers[i], NULL) == -1) {
signal(kExceptionSignals[i], SIG_DFL);
InstallDefaultHandler(kExceptionSignals[i]);
}
}
handlers_installed = false;
@@ -323,7 +341,7 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
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);
InstallDefaultHandler(sig);
}
pthread_mutex_unlock(&g_handler_stack_mutex_);
return;
@@ -340,14 +358,15 @@ void ExceptionHandler::SignalHandler(int sig, siginfo_t* info, void* uc) {
// previously installed handler. Then, when the signal is retriggered, it will
// be delivered to the appropriate handler.
if (handled) {
signal(sig, SIG_DFL);
InstallDefaultHandler(sig);
} else {
RestoreHandlersLocked();
}
pthread_mutex_unlock(&g_handler_stack_mutex_);
if (info->si_pid || sig == SIGABRT) {
// info->si_code <= 0 iff SI_FROMUSER (SI_FROMKERNEL otherwise).
if (info->si_code <= 0 || sig == SIGABRT) {
// This signal was triggered by somebody sending us the signal with kill().
// In order to retrigger it, we have to queue a new signal by calling
// kill() ourselves. The special case (si_pid == 0 && sig == SIGABRT) is