-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
pkg/fuzzer: use layered queues #4762
Conversation
7a1b19c
to
20a7018
Compare
While working on moving code from the target to the host, I see lots of utilities that duplicate code and pain to update.
All of these are similar: do something else (rather than pkg/fuzzer) after the VM checking procedure. |
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.
Will this allow to create the fuzzer lazily after the VM checking procedure?
For that we need some dynamically configurable layer that would The somewhat problematic consequence of the PR is that all running jobs of the same type get to execute something in a round robin manner. It doesn't matter for hints/smash/candidate, but it's somewhat problematic for triage jobs, because now it takes ages to see corpus/coverage increases after we have begun fuzzing. It looks like we should still let the triage jobs that we started earlier finish first. |
4f1689f
to
7fd743e
Compare
I've had to return priority queues to add order to the triage job executions. Now the question is whether these changes make
It feels like it's not just the execution queues that are common here, but rather taking requests from the queue and executing them on a pool of VMs -- that's what Repro and image testing are somewhat simpler of these 4 (if any VM crashed at any stage in the process, we don't retry, but just report the result), but syz-crush and runtest are indeed quite similar to what Syz-manager makes things a bit more complicated by dynamically resizing this VM pool to reproduce bugs. I'm not sure whether merging all these tools into syz-manager is a good idea, but taking this functionality to e.g. a separate package and using the package in all of those cases may be a good first step. |
7fd743e
to
1c1eb66
Compare
Potentially we could always create all layers statically at start, but then some layers (fuzzer) won't be ready to work until they receive additional bits of information, which will need to be passed to them later somehow. |
Significant part of common functionality even with image testing and repro is all of the VM booting and RPC communication (initial handshake, VM checking + normal exchanges). |
Potentially we could merge repro into the normal manager operation, repro will just supply more special requests for execution. Is special-ness is that they need to run on freshly booted VMs only. We needed that for syz-verifier (for final states of bug triage we also wanted to run tests of clean VMs). |
Agree that would be cleaner. However, it's even more work and refactoring. |
I've pushed the changes that move |
Now you feel part of my pain with all these duplicate tools :) |
|
ea47191
to
0a86ab0
Compare
0a86ab0
to
a244f3b
Compare
a244f3b
to
23358bb
Compare
4563b65
to
b4a10c5
Compare
Instead of relying on a fuzzer-internal priority queue, utilize stackable layers of request-generating steps. Move the functionality to a separate pkg/fuzzer/queue package. The pkg/fuzzer/queue package can be reused to add extra processing layers on top of the fuzzing and to combine machine checking and fuzzing execution pipelines.
Make Result statuses more elaborate. Instead of retrying inputs directly in rpc.go, extract this logic to a separate entity in pkg/fuzzer/queue.
Use the same interfaces as the fuzzer. Now syz-manager no longer needs to treat machine check executions differently.
There's no need to duplicate the execution mechanisms.
There's no need to wait until all results have been completed to print them.
Mark some requests as Important. The Retry() layer will give them one more chance even if they were not executed due to a VM crash. For now, the only important requests are related to triage, candidates and pkg/vminfo tests. Add tests for retry.go.
If syz-runtest wants several runs, it will pass it as an option for C code generation.
There's no need in duplicating the signal, coverage, hints flags.
For now, only ProgTypes is enough.
Return a new queue.Source from the machine check callback.
We don't support them in syz-manager.
Use a simpler implementation. Don't assume the nested Source may be nil.
b4a10c5
to
36b9899
Compare
Instead of relying on a fuzzer-internal priority queue, utilize stackable layers of request-generating steps.
Move the functionality to a separate pkg/fuzzer/queue package.
The pkg/fuzzer/queue package can be reused to add extra processing layers on top of the fuzzing and to combine machine checking and fuzzing execution pipelines.