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

wireshark: move building fuzzers to wireshark repository. #544

Merged
merged 1 commit into from
Apr 24, 2017

Conversation

jwzawadzki
Copy link
Contributor

as advised in #533 (comment) moving fuzzer build logic to wireshark repository.

@jwzawadzki
Copy link
Contributor Author

jwzawadzki commented Apr 23, 2017

It seems we have some license problems - Apache License 2.0 is currently not allowed in wireshark repository.
Can I relicense https://code.wireshark.org/review/#/c/21302/4/tools/oss-fuzzshark/build.sh to GPL-2 ?
// or any other allowed license: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob;f=tools/checklicenses.py;h=2e42e797ab566d1a80662be27b6256e6135ea747;hb=HEAD#l56

@inferno-chromium
Copy link
Collaborator

inferno-chromium commented Apr 24, 2017

build.sh will live in your repo, it can have any license you like to have there. once it lands there, let us know and we can merge this CL.

crondaemon pushed a commit to crondaemon/wireshark that referenced this pull request Apr 24, 2017
(oss-fuzz part google/oss-fuzz#544)

Change-Id: I54cf7a7b1aaa49582b5fff8bd034187aa6a9bdec
Reviewed-on: https://code.wireshark.org/review/21302
Petri-Dish: Jakub Zawadzki <darkjames-ws@darkjames.pl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Jakub Zawadzki <darkjames-ws@darkjames.pl>
@jwzawadzki
Copy link
Contributor Author

Patch merged upstream, build.sh relicenced to GPLv2. This can be merged as well.

@oliverchang oliverchang merged commit 68dfd93 into google:master Apr 24, 2017
for dissector in $FUZZ_MEDIA_TYPE_DISSECTORS; do
generate_fuzzer "media_type-${dissector}" "-DFUZZ_DISSECTOR_TABLE=\"media_type\" -DFUZZ_DISSECTOR_TARGET=\"$dissector\""
done
$SRC/wireshark/tools/oss-fuzzshark/build.sh all
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine as is, but I would encourage you to make one more step
and make the fuzz targets build as part of make so that they are always built and tested.
You don't have to link them against libFuzzer or other fizzing engines -- instead you may link them against something like https://github.com/llvm-mirror/llvm/tree/master/lib/Fuzzer/standalone
Then you'll be able to use those build targets for reproducing oss-fuzz bug reports and for regression testing,
w/o having to call this .sh script.

More: https://github.com/google/oss-fuzz/blob/master/docs/ideal_integration.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change to integrate fuzzer to wireshark build system is pending completion and review: https://code.wireshark.org/review/21064/. Original fuzzer is more interactive - it can list all targets, and is using environment variables to select target (instead of prepreocessor defines in oss version).

I am aware that current situation is not perfect, like build fails (https://oss-fuzz-build-logs.storage.googleapis.com/log-472514ce-1a41-4f14-8798-d45084212a65.txt) [fixed upstream], still bigger changes in build system -> more users are affected, it need to be discussed, and changes in tools/oss-fuzzshark/ can be merged fast.

@jwzawadzki jwzawadzki deleted the wireshark-move-build-fuzzers branch April 24, 2017 21:54
tmatth pushed a commit to tmatth/oss-fuzz that referenced this pull request Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants