six-leo / google-breakpad

Automatically exported from code.google.com/p/google-breakpad
0 stars 0 forks source link

Breakpad's use of sigaltstack() only applies to the first thread #374

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Breakpad's src/client/linux/handler/exception_handler.cc sets up a
signal stack using sigaltstack() and has the comment:
  // We run the signal handlers on an alternative stack because we might have
  // crashed because of a stack overflow.

However, signal stacks are per-thread, and there is nothing to set a
signal stack in any new threads.  This is probably not critical: the
signal handler will probably not go wrong unless the crash was caused
by running out of stack or (less likely) thread A is in the process of
underrunning a buffer on thread B's stack while thread B crashes.

However, it seems misleading to set up a signal stack if it's not
always used.

Original issue reported on code.google.com by mseaborn@chromium.org on 24 Mar 2010 at 4:19

GoogleCodeExporter commented 8 years ago
BTW the context is that, in Native Client, we must set up a signal stack in any 
thread 
that runs untrusted code, because we can't trust the contents of %esp/%rsp (see 
http://code.google.com/p/nativeclient/issues/detail?id=206).

Original comment by mseaborn@chromium.org on 24 Mar 2010 at 4:21

GoogleCodeExporter commented 8 years ago
This causes problems on Android unfortunately; bionic sets a sigaltstack for 
every thread at creation (in pthread_create) of size SIGSTKSZ, which is 8KiB 
for all relevant architectures. The main thread where we initialise breakpad in 
chrome/webview gets a larger stack created by breakpad's init code, but if we 
take a signal on another thread, it runs on the 8KiB bionic-created stack 
rather than running on the default stack as expected, and then we immediately 
overflow the stack because the handler function expects to be able to allocate 
a context on the stack and there isn't enough room - the kernel has already 
consumed more than half the stack with the original context. :( 

Original comment by torne@chromium.org on 18 Sep 2015 at 10:40

GoogleCodeExporter commented 8 years ago
+mark, you definitely want to take this into account for Crashpad.

I'm going to send a fix also for the current breakpad exception handler. 
Essentially, there is a high chance that most of the 64-bit crashes that happen 
on a thread != main are getting lost. the context struct is ~5k on aarch64, we 
get 8k of alt stack, and breakpad puts a copy (other than the one that the 
kernel pushes) on the stack.

Assigning this to me to fix it in breakpad.

Original comment by primi...@chromium.org on 18 Sep 2015 at 10:49