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

Verifier integration #570

Merged
merged 4 commits into from
Oct 20, 2024
Merged

Conversation

Alan-Jowett
Copy link
Collaborator

This pull request integrates the eBPF verifier as a submodule and updates the libfuzzer harness to utilize the verifier. The changes include modifications to submodule configurations, build scripts, and the addition of new verification and debugging functionalities in the libfuzzer harness.

Integration of eBPF Verifier:

Build Script Updates:

Enhancements to libfuzzer Harness:

  • libfuzzer/libfuzz_harness.cc:
    • Included headers for the eBPF verifier and platform.
    • Added verify_bpf_byte_code function to verify BPF bytecode using the eBPF verifier.
    • Implemented context management and debug functions for the uBPF context.
    • Integrated verification and debug functions into the interpreter and JIT execution paths. [1] [2] [3]
    • Added a call to verify_bpf_byte_code in the main fuzzer test function.

API Changes:

  • vm/inc/ubpf.h: Added register_mask parameter to the debug function signature.
  • vm/ubpf_vm.c: Updated the call to the debug function to include register_mask.

@Alan-Jowett Alan-Jowett force-pushed the verifier_integration branch 2 times, most recently from 89e5c7c to 3d34b60 Compare October 15, 2024 22:58
@coveralls
Copy link

coveralls commented Oct 15, 2024

Coverage Status

coverage: 79.503% (-0.1%) from 79.637%
when pulling 9ae1e5e on Alan-Jowett:verifier_integration
into 0642da6 on iovisor:main.

@Alan-Jowett Alan-Jowett requested a review from hawkinsw October 16, 2024 00:45
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
@Alan-Jowett Alan-Jowett force-pushed the verifier_integration branch 3 times, most recently from b2316bd to 0e15c81 Compare October 16, 2024 17:12
Copy link
Collaborator

@hawkinsw hawkinsw left a comment

Choose a reason for hiding this comment

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

I will review more as soon as I can, but I thought that I could send along these tiny little bits of feedback now! Thank you for doing this work -- I think that it is going to be tremendously helpful!

libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
vm/inc/ubpf.h Outdated Show resolved Hide resolved
vm/inc/ubpf.h Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
libfuzzer/libfuzz_harness.cc Outdated Show resolved Hide resolved
vm/ubpf_vm.c Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Alan Jowett added 3 commits October 18, 2024 09:48
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett <[email protected]>
Copy link
Collaborator

@hawkinsw hawkinsw left a comment

Choose a reason for hiding this comment

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

I think that there is a problem with our CMake configuration when the developer wants to have both "regular" tests and "fuzzing" tests on Linux. In that configuration, I get a linker error because vm/test.c has a main and then the libfuzzer (which is linked) has a main, too. I will try to do some debugging and see whether my hunch is true.

@hawkinsw
Copy link
Collaborator

I think that there is a problem with our CMake configuration when the developer wants to have both "regular" tests and "fuzzing" tests on Linux. In that configuration, I get a linker error because vm/test.c has a main and then the libfuzzer (which is linked) has a main, too. I will try to do some debugging and see whether my hunch is true.

I have confirmed that this "cross up" is the problem -- when the user (on Linux) has conformance tests and fuzzing tests enabled at the same time, there is a linker error.

There also seems to be a need to do an additional sanity test in cmake/settings.cmake -- it has a test for whether the CC is set to clang (for fuzzing) but does not guarantee that CXX is also properly set. I accidentally set CC to clang but had CXX set to g++ and I got a linker error re:

c++: error: unrecognized argument to ‘-fsanitize=’ option: ‘fuzzer’

It was easily correctable, but I loved what you did with the detection for the C compiler and thought we could do the same with the C++ compiler.

@Alan-Jowett Alan-Jowett merged commit 2d030d0 into iovisor:main Oct 20, 2024
48 checks passed
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.

4 participants