canistation / address-sanitizer

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

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

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
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.

Original issue reported on code.google.com by ramosian.glider@gmail.com on 16 Apr 2012 at 8:31

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

Original comment by ramosian.glider@gmail.com on 16 Apr 2012 at 8:34

GoogleCodeExporter commented 9 years ago
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)

Original comment by ramosian.glider@gmail.com on 16 Apr 2012 at 8:35

GoogleCodeExporter commented 9 years ago
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.

Original comment by ramosian.glider@gmail.com on 16 Apr 2012 at 10:25

GoogleCodeExporter commented 9 years ago

Original comment by ramosian.glider@gmail.com on 29 Oct 2012 at 11:46

GoogleCodeExporter commented 9 years ago
is this actionable? 

Original comment by konstant...@gmail.com on 15 Feb 2013 at 2:27

GoogleCodeExporter commented 9 years ago
Please reopen if we still need to do something. 

Original comment by konstant...@gmail.com on 18 Feb 2013 at 6:55

GoogleCodeExporter commented 9 years ago
Adding Project:AddressSanitizer as part of GitHub migration.

Original comment by ramosian.glider@gmail.com on 30 Jul 2015 at 9:12