syzkaller icon indicating copy to clipboard operation
syzkaller copied to clipboard

pkg/vminfo: move feature checking to host

Open dvyukov opened this issue 1 year ago • 2 comments

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 #1541

dvyukov avatar May 09 '24 12:05 dvyukov

Rebasing it to get the test coverage details.

tarasmadan avatar May 10 '24 12:05 tarasmadan

Codecov Report

Attention: Patch coverage is 51.14007% with 150 lines in your changes are missing coverage. Please review.

Project coverage is 61.3%. Comparing base (94b087b) to head (1666a9b).

Additional details and impacted files
Files Coverage Δ
pkg/vminfo/linux_syscalls.go 63.4% <ø> (-2.6%) :arrow_down:
pkg/vminfo/vminfo.go 77.4% <100.0%> (+1.1%) :arrow_up:
syz-fuzzer/testing.go 0.0% <0.0%> (ø)
syz-fuzzer/proc.go 0.0% <0.0%> (ø)
pkg/vminfo/netbsd.go 62.5% <62.5%> (ø)
pkg/vminfo/openbsd.go 57.1% <57.1%> (ø)
pkg/vminfo/syscalls.go 79.8% <84.0%> (+0.9%) :arrow_up:
pkg/ipc/ipc.go 48.3% <0.0%> (-0.4%) :arrow_down:
syz-manager/rpc.go 0.0% <0.0%> (ø)
syz-fuzzer/fuzzer.go 4.0% <0.0%> (+0.2%) :arrow_up:
... and 1 more

... and 2 files with indirect coverage changes

codecov[bot] avatar May 10 '24 12:05 codecov[bot]

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).

Correct.

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.

csource.Features is not just subset of host/flatrpc.Features, they also contain some unique ones. I don't know why it's in pkg/csource, I would say it should not stay anywhere at all :)

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.

Yes, we need 1 set of features that should cover all of them. The single set of features must be in a low-level package to avoid import cycles and also C code will need access to them. So flatrpc features looks like the best candidate. Flatbuffers generated code already allows some reflection (iterate over all flags, convert to string names). I think we should attach more meta info so that they can be used in all other places (e.g. which features are supported on which OSes, which features are only used by C source).

dvyukov avatar May 15 '24 13:05 dvyukov