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

Dont consider it a crash when CF stops libFuzzer. #2471

Merged
merged 2 commits into from
Oct 7, 2021
Merged

Conversation

jonathanmetzman
Copy link
Collaborator

@jonathanmetzman jonathanmetzman commented Oct 5, 2021

LibFuzzer sometimes needs to be stopped on its own by CF. CF
does this by sending libFuzzer a SIGTERM. Ordinarily, this causes
libFuzzer to return 72.
(see https://github.com/llvm/llvm-project/blob/1f161919065fbfa2b39b8f373553a64b89f826f8/compiler-rt/lib/fuzzer/FuzzerOptions.h#L25)

Don't consider 72 to be a crashing return code.
Fixes: google/oss-fuzz#6557

@google-cla google-cla bot added the cla: yes CLA signed. label Oct 5, 2021
@jonathanmetzman
Copy link
Collaborator Author

/gcbrun

@oliverchang
Copy link
Collaborator

Thanks! I think you need to rebase to get the CI to pass.

Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change itself LGTM.

@@ -300,7 +300,8 @@ def fuzz(self, target_path, options, reproducers_dir, max_time):
# If we exited with a non-zero return code with no crash file in output from
# libFuzzer, this is most likely a startup crash. Use an empty testcase to
# to store it as a crash.
if not crash_testcase_file_path and fuzz_result.return_code:
if (not crash_testcase_file_path and
fuzz_result.return_code not in constants.NONCRASH_RETURN_CODES):
crash_testcase_file_path = self._create_empty_testcase_file(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also just change this to create file with less restrictive permissions. There's no reason to lock it down.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I agree. I think there are security reasons not to do this.

@evverx
Copy link

evverx commented Oct 7, 2021

Thanks! It would be great to get this PR merged. Without it CIFuzz always fails when report-unreproducible-crashes is set to true

LibFuzzer sometimes needs to be stopped on its own by CF. CF
does this by sending libFuzzer a SIGTERM. Ordinarily, this causes
libFuzzer to return 72.
(see https://github.com/llvm/llvm-project/blob/1f161919065fbfa2b39b8f373553a64b89f826f8/compiler-rt/lib/fuzzer/FuzzerOptions.h#L25)

Don't consider 72 to be a crashing return code.
@jonathanmetzman
Copy link
Collaborator Author

/gcbrun

@jonathanmetzman
Copy link
Collaborator Author

Thanks! It would be great to get this PR merged. Without it CIFuzz always fails when report-unreproducible-crashes is set to true

Trying to do this today.

@jonathanmetzman
Copy link
Collaborator Author

/gcbrun

@jonathanmetzman jonathanmetzman enabled auto-merge (squash) October 7, 2021 19:20
@jonathanmetzman jonathanmetzman merged commit 89f0fde into master Oct 7, 2021
@jonathanmetzman jonathanmetzman deleted the exit-code branch October 7, 2021 19:31
@jonathanmetzman
Copy link
Collaborator Author

@oliverchang Is the pip package updated automatically? How can I release a new version to include this change.

@oliverchang
Copy link
Collaborator

@oliverchang Is the pip package updated automatically? How can I release a new version to include this change.

We need to make a new release. I'll do it.

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

Successfully merging this pull request may close these issues.

Uploading artifacts fails with Access to the path is denied
3 participants