-
Notifications
You must be signed in to change notification settings - Fork 1.4k
KFuzzTest integration #6280
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
KFuzzTest integration #6280
Conversation
tarasmadan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do more batches later.
1e61119 to
d15ea9d
Compare
|
@gemini /review |
|
@gemini-cli /review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
This pull request is a solid proof-of-concept for integrating KFuzzTest into syzkaller. The overall structure is well-thought-out, with clear separation of concerns between extracting KFuzzTest definitions, building syzlang descriptions, and executing the tests.
🔍 General Feedback
- The main point of feedback is the extensive use of
panicthroughout the new code. While this might be acceptable for a proof-of-concept, it should be replaced with proper error handling to make the code more robust and suitable for production use, especially in library code. - There are a few places where constants are hardcoded (e.g., file paths, syscall IDs). It would be better to define these in a central place to improve maintainability.
- The code is generally well-written and easy to follow. The addition of documentation and a standalone tool for generating descriptions is a great touch.
Overall, this is a great start, and with some improvements to error handling and configuration, it will be a valuable addition to syzkaller.
89b70a6 to
444f025
Compare
ff828c0 to
f533625
Compare
944e267 to
2184b15
Compare
497948e to
aed938f
Compare
8997cd1 to
81f2154
Compare
7c1b377 to
6d82ba9
Compare
Add a Go-native KCOV package, with a helper functions for tracing a a function. This is in preparation for a standalone KFuzzTest tool, which should be written in Go in order to take advantage of existing fuzzing infrastructure. The hard-coded coverage buffer size is the same as the executor program, defined as `512 << 10` in `executor/executor.cc`. Signed-off-by: Ethan Graham <ethangraham@google.com>
Add syz_kfuzztest_run pseudo-syscall, KFuzzTest attribute, and encoding logic. KFuzzTest targets, which are invoked in the executor with the new syz_kfuzztest_run pseudo-syscall, require specialized encoding. To differentiate KFuzzTest calls from standard syzkaller calls, we introduce a new attribute called KFuzzTest or "kfuzz_test" in syzkaller descriptions that can be used to annotate calls. Signed-off-by: Ethan Graham <ethangraham@google.com>
As KFuzzTest targets are discovered at boot, we need a mechanism for adding these to the array of enabled system calls. This is implemented by the new Extend method, which performs this setup. Signed-off-by: Ethan Graham <ethangraham@google.com>
All non-base variants of syz_kfuzztest_run (i.e., those that are discovered dynamically) are encoded so that they map onto the base variant which is defined in kfuzztest.txt, and known by the executor. We add a function for fetching this, that is wrapped in a sync.once block to avoid repeated iteration over the target's array of syscalls. Signed-off-by: Ethan Graham <ethangraham@google.com>
Add a new package, pkg/kfuzztest, that implements dynamic discovery of KFuzzTest targets by parsing a vmlinux kernel binary. Signed-off-by: Ethan Graham <ethangraham@google.com>
Internal kernel functions (and as a result KFuzzTest) have stricter contracts than system calls. For this reason, we must avoid mutating the following cases: - Length arguments not matching the length of the related buffer. - Strings not being null-terminated. Add special cases for KFuzzTest calls that avoids these situations. Signed-off-by: Ethan Graham <ethangraham@google.com>
Add logic for dynamic KFuzzTest target discovery in syz-manager. By default, all KFuzzTest targets are enabled when the enable_kfuzztest config option is set to true.
Signed-off-by: Ethan Graham <ethangraham@google.com>
syz-kfuzztest is a new standalone designed for fuzzing KFuzzTest on a live kernel VM (e.g., inside QEMU). It has no dependencies on the executor program, instead directly writing into a KFuzzTest target's debugfs entry. Signed-off-by: Ethan Graham <ethangraham@google.com>
Add a tool for generating a syscaller description for every KFuzzTest target discovered in a vmlinux binary and outputting it to stdout. Signed-off-by: Ethan Graham <ethangraham@google.com>
Add documentation for syzkaller's KFuzzTest integration, and a separate documentation file for the syz-kfuzztest program. Signed-off-by: Ethan Graham <ethangraham@google.com>
Introduce a KFuzzTest mode for the fuzzer so that a smaller number of recommended calls can be used if we are fuzzing KFuzzTest targets. Signed-off-by: Ethan Graham <ethangraham@google.com>
If vmlinux is specified as a flag, we perform a setup stage where we parse vmlinux for KFuzzTest targets. Signed-off-by: Ethan Graham <ethangraham@google.com>
6d82ba9 to
b755cbe
Compare
Previously, the generated KFuzzTest programs were reusing the address of the top-level input struct. A problem could arise when the encoded blob is large and overflows into another allocated region - this certainly happens in the case where the input struct points to some large char buffer, for example. While this wasn't directly a problem, it could lead to racy behavior when running KFuzzTest targets concurrently. To fix this, we now introduce an additional buffer parameter into syz_kfuzztest_run that is as big as the maximum accepted input size in the KFuzzTest kernel code. When this buffer is allocated, we ensure that we have some allocated space in the program that can hold the entire encoded input. This works in practice, but has not been tested with concurrent KFuzzTest executions yet.
b755cbe to
849f9ff
Compare
|
Oh, I didn't notice it, thanks for reporting. |
Description
KFuzzTest integration in syzkaller.
Readme in
docs/kfuzztest.md,syz-kfuzztest-specific documentation indocs/syz-kfuzztest.md.Additions
syz-kfuzztest.syz-managerfor dynamic discovery and continuous fuzzing ofof KFuzzTest targets.
for KFuzzTest targets discovered from a
vmlinuxbinary.syz_kfuzztest_run's ID from a target (cached)Modifications
target.lazyInit()fetches KFuzzTest run's ID