Land LibFuzzer code to mozilla-central

RESOLVED FIXED in Firefox 53

Status

()

Core
General
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: decoder, Assigned: decoder)

Tracking

({sec-want})

Trunk
mozilla53
Unspecified
Linux
sec-want
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 wontfix, firefox53 fixed)

Details

(Whiteboard: [adv-main53-])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
+++ This bug was initially created as a clone of Bug #1289194 +++

Once bug 1289194 has landed, we will want to land the libfuzzer code itself to mozilla-central in order to allow enabling this code in certain testing builds (e.g. the ASan builds we have).

There are two changes we need to make to the code:

1) We need to include a full copy of LLVM's LICENSE.TXT in the root of the libfuzzer directory

2) We need to somehow disable LibFuzzer when the code is being built with afl-clang because they both hook into __sanitizer_cov_trace_pc. Alternatively, we could disable it on the AFL side because it's not used by default.
(Assignee)

Updated

2 years ago
Summary: Add LibFuzzer support for testing xul code → Land LibFuzzer code to mozilla-central
(Assignee)

Updated

2 years ago
Depends on: 1303757
(Assignee)

Comment 1

2 years ago
Step 2) is being handled by bug 1303757 now and therefore that bug should land first.
(Assignee)

Comment 2

2 years ago
Created attachment 8808221 [details] [diff] [review]
libfuzzer-import-v1.patch

This patch

1) Imports the libfuzzer code parts we need
2) Updates the clone_libfuzzer.sh script to delete all the parts that we don't need (e.g. tests)
3) Updates moz.build to include two files that were not in the version we used when we developed the initial libfuzzer integration.

I confirmed that my local build works using this code import.

For the license, I added LICENSE.TXT from the LLVM repository as it is to the directory to fulfill the licensing requirements (confirmed this with Gerv).
Assignee: nobody → choller
Status: NEW → ASSIGNED
Attachment #8808221 - Flags: review?(nfroyd)
Comment on attachment 8808221 [details] [diff] [review]
libfuzzer-import-v1.patch

Review of attachment 8808221 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the known version/hash change below.  I'd personally do the "copy only what we need" change, too, but I'm not going to insist on that.

::: tools/fuzzing/libfuzzer/clone_libfuzzer.sh
@@ +1,4 @@
>  #!/bin/sh
>  
>  mkdir tmp/
>  git clone --no-checkout --depth 1 https://chromium.googlesource.com/chromium/llvm-project/llvm/lib/Fuzzer tmp/

I don't know if we do this consistently in all of our import scripts (I know we do for jemalloc and rust mp4parse), but it would be good to either clone from a known version/hash, or |git checkout| a known version/hash after cloning.  The script is then more self-documenting about what we're importing.

@@ +6,4 @@
>  git reset --hard HEAD
> +
> +# Remove the temporary directory and anything else we don't need
> +rm -Rf tmp/ test/ afl/ standalone/ CMakeLists.txt cxx.dict build.sh README.txt

I know the script already kind of did things this way, but our import scripts typically copy in only the files we need (using wildcards if necessary to make things easier).  Copying in just what we need rather than indiscriminately copying, followed by removal, means that we're deliberately choosing the files that get brought in, and avoids cases where we might, e.g. overwrite the LICENSE.TXT we've manually imported.
Attachment #8808221 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 4

2 years ago
Created attachment 8809201 [details] [diff] [review]
libfuzzer-import-v2.patch

I fixed the import script to explicitly specify the revision that is checked out (this will break the script though once new commits are added because the clone is a shallow clone). I also only copy what is necessary (cpp, h and def files) now.
Attachment #8808221 - Attachment is obsolete: true
Attachment #8809201 - Flags: review?(nfroyd)
Comment on attachment 8809201 [details] [diff] [review]
libfuzzer-import-v2.patch

Review of attachment 8809201 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you!
Attachment #8809201 - Flags: review?(nfroyd) → review+

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/627c11c22312
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
We've built 51 RC. Mark 51 won't fix.
status-firefox51: affected → wontfix
Whiteboard: [adv-main53-]
You need to log in before you can comment on or make changes to this bug.