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/vminfo: move feature checking to host #4790
Conversation
With this commit + google#4790, syz-manager runs in host fuzzer mode (with SYZ_STARNIX_HACK=1) without getting stuck on feature setup. Co-authored-by: [email protected]
152bd72
to
227dda6
Compare
Rebasing it to get the test coverage details. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
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.
Oh, good luck to us in merging the changes from this PR and from #4762 :)
Here's the summary I've tried to write after looking at the code. Does it look correct?
Some features (1) require a one-time setup, other features (2) are passed via ipc.EnvFlags
and we must set them up on every syz-executor
restart. In syz-fuzzer
(actually, mostly in syz-executor
), we now try to enable all (1) and also do tests for some of (2). Then, pkg/vminfo
does more tests for (2).
There are also csource.Features
, which are at this point just a collection of string flags for a subset (?) of (1) and (2). In host.SetupFeatures
, we intersect it with the available features of type (1). In ipc.FeaturesToFlags
, we intersect it with the available features (2) to form EnvFlags
.
We don't seem to care about csource.Features
in the generation of C programs at all, so do csource.Features
have to stay in pkg/csource
?
There's also csource.Options
, which seems to both intersect with (1)/(2) and ipc.ExecOpts
. csource.Options
is only used for C repro generation and only determines what consts are substituted into the C code.
Interestingly, it seems that fuzzer.Request
(= queue.Request
from the other PR) is already moving to a direction of being a superset of all parameters we want to control when executing a program. Could it eventually cover also all of csource.Features
and csource.Features
and actually replace them?
I think it will be a very nice if we could both execute such a Request
on a syz-executor
and translate it to a C program without having to specify any more parameters.
afae8ac
to
af08937
Compare
Feature checking procedure is split into 2 phases: 1. syz-fuzzer invokes "syz-executor setup feature" for each feature one-by-one, and checks if executor does not fail. Executor can also return a special "this feature does not need custom setup", this allows to not call setup of these features in each new VM. 2. pkg/vminfo runs a simple program with ipc.ExecOpts specific for a concrete feature, e.g. for wifi injection it will try to run a program with wifi feature enabled, if setup of the feature fails, executor should also exit with an error. For coverage features we also additionally check that we actually got coverage. Then pkg/vminfo combines results of these 2 checks into final result. syz-execprog now also uses vminfo package and mimics the same checking procedure. Update google#1541
af08937
to
1666a9b
Compare
Correct.
Yes, we need 1 set of features that should cover all of them. |
Feature checking procedure is split into 2 phases:
and checks if executor does not fail.
Executor can also return a special "this feature does not need custom setup",
this allows to not call setup of these features in each new VM.
e.g. for wifi injection it will try to run a program with wifi feature enabled,
if setup of the feature fails, executor should also exit with an error.
For coverage features we also additionally check that we actually got coverage.
Then pkg/vminfo combines results of these 2 checks into final result.
syz-execprog now also uses vminfo package and mimics the same checking procedure.
Update #1541