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

oss-fuzz and fuzzer files #551

Merged
merged 12 commits into from
Nov 4, 2022
Merged

oss-fuzz and fuzzer files #551

merged 12 commits into from
Nov 4, 2022

Conversation

0x34d
Copy link
Contributor

@0x34d 0x34d commented Oct 9, 2022

Moving fuzzing files into the main project for better harness reliability.

Signed-off-by: 0x34d ajsinghyadav00@gmail.com

Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@vincentbernat
Copy link
Member

I prefer not to mix Apache 2 with ISC. This duplicates the code present in test/decode.c. You can use --enable-sanitizers=fuzzer to not alter the configuration system.

Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@0x34d
Copy link
Contributor Author

0x34d commented Oct 10, 2022

This duplicates the code present in test/decode.c.

Well, it's possible. 
But eventually, this will become a mess for the linker.

@0x34d
Copy link
Contributor Author

0x34d commented Oct 10, 2022

You can use --enable-sanitizers=fuzzer to not alter the configuration system.

This flag will break the compilation of binary with the main function. 
because the -fsanitize=fuzzer library also has a main function.

While we are using multiple types of fuzzer for fuzzing.
I have to be specific to the linker about the fuzzing engine library.

Local Fuzzing linker for FuzzDecode.c:

LibFuzzer=-fsanitize=fuzzer 
AFL++=FuzzingEngine.a
honggfuzz=libhfuzz.a

oss-fuzz fuzzing linker for FuzzDecode.c:

LibFuzzer=-fsanitize=fuzzer 
AFL++=libFuzzingEngine.a
honggfuzz=libFuzzingEngine.a

As per your concern about the configuration system.
Now I will use a different approach.

--enable-fuzzer will provide fuzzing support.

For the linker, LIB_FUZZING_ENGINE needs to be export LIB_FUZZING_ENGINE=fuzzing.a,
as per the corresponding fuzzer.

So configure.ac can read the linker for binary.

Regards Arjun.

Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@vincentbernat
Copy link
Member

I was thinking about something like this: 9fd7b25.

But if it's too much a hurdle, we can duplicate the code.

@0x34d
Copy link
Contributor Author

0x34d commented Oct 10, 2022

I was thinking about something like this: 9fd7b25.

But if it's too much a hurdle, we can duplicate the code.

Yes, This can work with a few changes in macros.

Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@0x34d
Copy link
Contributor Author

0x34d commented Oct 10, 2022

9fd7b25#diff-7238ff164833857def0655f4fe7742553e6e42bae79089c741a599bf8aa0b02dR76

Are you sure about this one:

	if (cdp_decode(NULL, frame, size, hardware, &nchassis, &nport) == -1) {

I believe this is the right way:

	if (cdp_decode(NULL, frame, size, hardware, nchassis, nport) == -1) {

Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@0x34d
Copy link
Contributor Author

0x34d commented Oct 10, 2022

You can review the PR.
I had some new ideas you can check them out.

@vincentbernat
Copy link
Member

Sorry for the late answer. I am OK with the code change, the changes to configure.ac and Makefile.am. However, I am a bit more worried with the script and the ZIP file. The script could go in the Makefile.

Also, from a style point of view, I prefer fuzz-decode to FuzzDecode. Everything else is using snake case, not camel case.

Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@vincentbernat vincentbernat merged commit 1bdefa8 into lldpd:master Nov 4, 2022
vincentbernat pushed a commit that referenced this pull request Nov 4, 2022
Signed-off-by: 0x34d <ajsinghyadav00@gmail.com>
@vincentbernat
Copy link
Member

Thanks. I have renamed and simplified a bit the build script.

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

2 participants