Need to agree on the behavior of SEGV handlers in ASan, Chromium and NaCl

[ramosian-glider]

Originally reported on Google Code with ID 65

Currently Chromium does some magic with the signal handlers in various processes.
1. The main browser process may set a handler for SIGSEGV, which is used by the child
processes (for example, Breakpad does this).
2. Because of http://code.google.com/p/nativeclient/issues/detail?id=1607, the Native
Client process forked by Chrome should not have any non-NaCl signal handlers installed.
When setting the Breakpad signal handler, this is handled correctly.
3. Some child processes spawned by Chrome may reset the SIGSEGV handler to SIG_DFL.

ASan runtime changes the game by installing its own SIGSEGV handler at program startup.

Prior to r154390 (http://llvm.org/viewvc/llvm-project?view=rev&revision=154390) we
just intercepted signal() and sigaction() and didn't allow the client to install its
own nandler for SEGV. This guaranteed that all the processes had the same signal handler,
but didn't play well with NaCl (it couldn't run the qualification tests because this
required setting a signal handler; also the process remained vulnerable, see above).

r154390 allowed the clients to set the signal handlers at their own risk. This allowed
child processes to set SIG_DFL as the SIGSEGV handler. As a result the NaCl tests started
to work (for them this was equivalent to running ASan with ASAN_OPTIONS="use_segv=0"),
but this has caused our coverage to drop, because we were unable to detect access violation
in Chromium processes that had reset their signal handlers.
Trying to remove the calls to "signal(SIGSEGV, SIG_DFL)" from the Chromium codebase
to fix the coverage led to NaCl being unhappy again: this time it could notice the
ASan handler and abort execution because of its unsafety.

Reported by ramosian.glider on 2012-04-16 08:31:41

[ramosian-glider]

r154803 reverts r154390 and disallows the clients to set their signal handlers.
This should help Chrome, although NaCl tests are broken again.

Reported by ramosian.glider on 2012-04-16 08:34:33

[ramosian-glider]

From the Chromium point of view ASan is a platform, and our SEGV handler is essentially
the default handler for this platform.
If we allow the clients to disable it at all, this may help NaCl, but we'll have to
remove all the Chromium code that disables it in the child processes, so that we don't
lose coverage.
At the moment such a change leads to NaCl being sad again, because it does not consider
ASan as a platform and does not want any signal handlers from it.
Therefore we need some changes on the NaCl side, so the question arises whether we
actually needed to change the Chrome part.

I see two possible solutions for the problem.
1. Let the clients disable our signal handler at their own risk.
Remove all the Chromium code that sets SIG_DFL for the child processes.
When the NaCl process is started, set the signal handler to SIG_DFL.

2. Keep being a platform.
Let the clients set their own signal handlers.
Choosing SIG_DFL sets the ASan handler back.
If the client (e.g. NaCl) really wants the system SIG_DFL, it must
pass some predefined value to sigaction()
(we can either define an additional SIG_ constant, or have some RTL
function with a special meaning, e.g. signal(SIGSEGV,__asan_system_default_signal_handler)
will set the handler to SIG_DFL)

Reported by ramosian.glider on 2012-04-16 08:35:57

[ramosian-glider]

In fact just calling some ASan interface function (__asan_reset_default_signal_handler)
is even better than using a special value to pass to signal/sigaction.

Reported by ramosian.glider on 2012-04-16 10:25:08

[ramosian-glider]

Reported by ramosian.glider on 2012-10-29 11:46:53

• Labels added: Chromium

[ramosian-glider]

is this actionable? 

Reported by konstantin.s.serebryany on 2013-02-15 14:27:01

[ramosian-glider]

Please reopen if we still need to do something. 

Reported by konstantin.s.serebryany on 2013-02-18 06:55:08

• Status changed: WontFix

[ramosian-glider]

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:59

• Labels added: ProjectAddressSanitizer