Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle targets that implement signal handlers #675

Closed
oliverchang opened this issue Jun 12, 2017 · 16 comments
Closed

Handle targets that implement signal handlers #675

oliverchang opened this issue Jun 12, 2017 · 16 comments
Labels

Comments

@oliverchang
Copy link
Collaborator

Targets that install their own SIGALRM/SIGSEGV/SIGABRT etc handlers may prevent libFuzzer/sanitizers from reporting timeouts/crashes in a way that ClusterFuzz understands.

#671 is an example -- libreoffice targets fail quickly on a timeout, but because there's a SIGALRM handlers that exit silently our logs show that the target just exits after some time without any indication why.

We should explore if there is a good way to prevent this situation.

@kcc
Copy link
Contributor

kcc commented Jun 13, 2017

Ughhh...
@vitalybuka has made a change recently where we stopped blocking users from redefining asan's sig handlers by default.

        handle_segv
                - Controls custom tool's SIGSEGV handler (0 - do not registers the handler, 1 - register the handler and allow user to set own, 2 - registers the handler and block user from changing it). 
        handle_sigbus
                - Controls custom tool's SIGBUS handler (0 - do not registers the handler, 1 - register the handler and allow user to set own, 2 - registers the handler and block user from changing it). 
        handle_abort
                - Controls custom tool's SIGABRT handler (0 - do not registers the handler, 1 - register the handler and allow user to set own, 2 - registers the handler and block user from changing it). 
        handle_sigill
                - Controls custom tool's SIGILL handler (0 - do not registers the handler, 1 - register the handler and allow user to set own, 2 - registers the handler and block user from changing it). 
        handle_sigfpe
                - Controls custom tool's SIGFPE handler (0 - do not registers the handler, 1 - register the handler and allow user to set own, 2 - registers the handler and block user from changing it). 

so, for those signals the simplest is to set all these flags to 2

As for SIGALRM... Hmmm. asan has nothing to do with SIGALRM.
libFuzzer needs to have it's own SIGALRM but it has no way to block the user from redefining it.

@kcc
Copy link
Contributor

kcc commented Jun 13, 2017

we can also add a code to libFuzzer to report a timeout if a single input took more than X seconds
even if the ALRM did not fire. This will work against slow inputs (that take > X but eventuall finish), but won't work against infinite loops (i.e. it's a partial solution)

@kcc
Copy link
Contributor

kcc commented Jun 13, 2017

here is a proposal:

  • support SIGALRM interception in sanitizers
  • by default, set handle_alrm to 0
  • on handle_alrm > 0, install a handler that calls a user-provided callback (via __sanitizer_set_sigalrm_callback)

@vitalybuka WDYT?

@vitalybuka
Copy link
Contributor

Why not in libFuzzer? It looks very libFuzzer specific.

@kcc
Copy link
Contributor

kcc commented Jun 13, 2017

sanitizers have the interception machinery, which is too hairy to have it re-implemented in libFuzzer

@oliverchang
Copy link
Collaborator Author

Re handle_signal=2, which llvm revision introduced it?

For our current builds with ASAN_OPTIONS=help=1, I'm not seeing "2" in the documentation:

        handle_segv
                - Controls custom tool's SIGSEGV handler (0 - do not registers the handler, 1 - register the handler).

Will it break if we set handle_signal=2 before we pick up the llvm change?

@vitalybuka
Copy link
Contributor

It was introduced in r303941

@vitalybuka
Copy link
Contributor

before r303941 you can set allow_user_segv_handler=0

@kcc
Copy link
Contributor

kcc commented Jun 14, 2017

See also https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=1916#c7
where our recent changes in asan are causing trouble (sorry!)

I wonder if we can bump the llvm revision manually.
Since recently we started following the chrome's clang revision,
but maybe we should simply ping a revision of our own and periodically bump it?

@oliverchang
Copy link
Collaborator Author

I wonder if we can bump the llvm revision manually.
Since recently we started following the chrome's clang revision,
but maybe we should simply ping a revision of our own and periodically bump it?

It looks chromium llvm just got a bump to r305281: https://chromium.googlesource.com/chromium/src/+/3a26e05c70e63815c7c18284e5e006b9d0ac75af

I'm guessing this is why the bug got marked as fixed just today? (because we don't have handle_signal=2) ?

Let's explore pinning our own revision if this keeps coming up.. clang bumps in Chrome are usually done after a bunch of tests, so it would be nice if we could just piggy back off that.

@kcc
Copy link
Contributor

kcc commented Jun 14, 2017

One alternative: bump the revisions manually, but only to the revisions tested by Chrome (unless there is a urgency like this one)

@vitalybuka
Copy link
Contributor

Actually I don't see this as revision issues.
Here manual replacing handle_signal=1 to handle_signal=2 was required anyway.
So it's rather my fault to notify oss-fuzz and chromium. I am very sorry for that.

@oliverchang
Copy link
Collaborator Author

oliverchang commented Jun 14, 2017

argh, unfortunately we cannot just set handle_signal=2 everywhere, since we still need to run older builds for regression and fixed testing, and setting handle_signal=2 isn't backwards compatible and will break those things..... is there anything that can be done on the sanitizers side to make this feature more backwards compatible?

For now we may need to roll back the clang revision, or wait for libFuzzer to be less aggressive about installing signal handlers.

@oliverchang
Copy link
Collaborator Author

Discussed offline with @vitalybuka

We need sanitizer options to be backwards compatible across builds. with handle_signal=2 unfortunately if we pass it to an older build we get a hard failure:

ERROR: Invalid value for bool option: '2'
ERROR: Flag parsing failed.

Adding new flags should be fine, because if we pass it to an older build we don't get a hard failure.

@vitalybuka said he will make a compatibility fix soon

Tracking this issue chromium side in https://bugs.chromium.org/p/chromium/issues/detail?id=731130#c10. If we don't get a revert there I'll manually pin our revision to the previous one.

dtzWill pushed a commit to llvm-mirror/compiler-rt that referenced this issue Jun 15, 2017
Summary:
After r303941 it was not possible to setup ASAN_OPTIONS to have the same
behavior for pre r303941 and post r303941 builds.
Pre r303941 Asan does not accept handle_sigbus=2.
Post r303941 Asan does not accept allow_user_segv_handler.

This fix ignores allow_user_segv_handler=1, but for allow_user_segv_handler=0
it will upgrade flags like handle_sigbus=1 to handle_sigbus=2. So user can set
ASAN_OPTIONS=allow_user_segv_handler=0 and have same behavior on old and new
clang builds (except range from r303941 to this revision).

In future users which need to prevent third party handlers should switch to
handle_sigbus=2 and remove allow_user_segv_handler as soon as suport of older
builds is not needed.

Related bugs:
  google/oss-fuzz#675
  https://bugs.chromium.org/p/chromium/issues/detail?id=731130

Reviewers: eugenis

Subscribers: llvm-commits, kubamracek

Differential Revision: https://reviews.llvm.org/D34227

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@305433 91177308-0d34-0410-b5e6-96231b3b80d8
@inferno-chromium
Copy link
Collaborator

What is the plan here ?

@nsajko
Copy link

nsajko commented Aug 23, 2019

Is the solution for this not for libFuzzer to do either of these things (that require POSIX signal.h and time.h) instead of using SIGALRM or other common signals:

  • use SIGEV_THREAD with timer_create and timer_settime

  • look for available signals (after the fuzzer starts) in the range SIGRTMIN:SIGRTMAX using sigaction and use what you find

  • use a couple signals on the edge of the SIGRTMIN:SIGRTMAX range and then adjust that range with an interceptor function (like AddressSanitizer uses for malloc, etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants