From d4480042e9dcaa1d1a71222c8056a0b9b9c5bd9a Mon Sep 17 00:00:00 2001 From: Alexander Potapenko Date: Mon, 2 Dec 2024 15:27:50 +0100 Subject: syz-manager: pkg: skip seeds with mismatching requirements in fuzzing mode When running in the test mode, syz-manager already ignores tests that have arch requirements mismatching the target arch. Because the same tests are also used as seeds in the fuzzing mode, skip them likewise, instead of reporting errors if they contain arch-specific syscalls. The code and tests for parsing the requirements is moved from pkg/runtest to pkg/manager. --- pkg/manager/seeds.go | 101 +++++++++++++++++++++++++++++++++++++++++----- pkg/manager/seeds_test.go | 29 +++++++++++++ pkg/runtest/run.go | 62 ++-------------------------- pkg/runtest/run_test.go | 21 ---------- 4 files changed, 124 insertions(+), 89 deletions(-) create mode 100644 pkg/manager/seeds_test.go (limited to 'pkg') diff --git a/pkg/manager/seeds.go b/pkg/manager/seeds.go index 3a83a0788..2d769d43b 100644 --- a/pkg/manager/seeds.go +++ b/pkg/manager/seeds.go @@ -4,11 +4,15 @@ package manager import ( + "bufio" + "bytes" + "errors" "fmt" "math/rand" "os" "path/filepath" "runtime" + "strings" "sync" "time" @@ -94,13 +98,19 @@ func LoadSeeds(cfg *mgrconfig.Config, immutable bool) Seeds { close(inputs) }() brokenSeeds := 0 + skippedSeeds := 0 var brokenCorpus []string var candidates []fuzzer.Candidate for inp := range outputs { if inp.Prog == nil { if inp.IsSeed { - brokenSeeds++ - log.Logf(0, "seed %s is broken: %s", inp.Path, inp.Err) + if errors.Is(inp.Err, ErrSkippedTest) { + skippedSeeds++ + log.Logf(2, "seed %s is skipped: %s", inp.Path, inp.Err) + } else { + brokenSeeds++ + log.Logf(0, "seed %s is broken: %s", inp.Path, inp.Err) + } } else { brokenCorpus = append(brokenCorpus, inp.Key) } @@ -123,6 +133,9 @@ func LoadSeeds(cfg *mgrconfig.Config, immutable bool) Seeds { if len(brokenCorpus)+brokenSeeds != 0 { log.Logf(0, "broken programs in the corpus: %v, broken seeds: %v", len(brokenCorpus), brokenSeeds) } + if skippedSeeds != 0 { + log.Logf(0, "skipped %v seeds", skippedSeeds) + } if !immutable { // This needs to be done outside of the loop above to not race with corpusDB reads. for _, sig := range brokenCorpus { @@ -179,28 +192,96 @@ func versionToFlags(version uint64) fuzzer.ProgFlags { } func ParseSeed(target *prog.Target, data []byte) (*prog.Prog, error) { - return parseProg(target, data, prog.NonStrict) + p, _, err := parseProg(target, data, prog.NonStrict, nil) + return p, err +} + +func ParseSeedWithRequirements(target *prog.Target, data []byte, reqs map[string]bool) ( + *prog.Prog, map[string]bool, error) { + return parseProg(target, data, prog.Strict, reqs) } -func ParseSeedStrict(target *prog.Target, data []byte) (*prog.Prog, error) { - return parseProg(target, data, prog.Strict) +func parseRequires(data []byte) map[string]bool { + requires := make(map[string]bool) + for s := bufio.NewScanner(bytes.NewReader(data)); s.Scan(); { + const prefix = "# requires:" + line := s.Text() + if !strings.HasPrefix(line, prefix) { + continue + } + for _, req := range strings.Fields(line[len(prefix):]) { + positive := true + if req[0] == '-' { + positive = false + req = req[1:] + } + requires[req] = positive + } + } + return requires +} + +func checkArch(requires map[string]bool, arch string) bool { + for req, positive := range requires { + const prefix = "arch=" + if strings.HasPrefix(req, prefix) && + arch != req[len(prefix):] == positive { + return false + } + } + return true } -func parseProg(target *prog.Target, data []byte, mode prog.DeserializeMode) (*prog.Prog, error) { +func MatchRequirements(props, requires map[string]bool) bool { + for req, positive := range requires { + if positive { + if !props[req] { + return false + } + continue + } + matched := true + for _, req1 := range strings.Split(req, ",") { + if !props[req1] { + matched = false + } + } + if matched { + return false + } + } + return true +} + +var ErrSkippedTest = errors.New("skipped test based on constraints") + +func parseProg(target *prog.Target, data []byte, mode prog.DeserializeMode, reqs map[string]bool) ( + *prog.Prog, map[string]bool, error) { + properties := parseRequires(data) + // Need to check requirements early, as some programs may fail to deserialize + // on some arches due to missing syscalls. We also do not want to parse tests + // that are marked as 'manual'. + if !checkArch(properties, target.Arch) || !MatchRequirements(properties, reqs) { + var pairs []string + for k, v := range properties { + pairs = append(pairs, fmt.Sprintf("%s=%t", k, v)) + } + return nil, properties, fmt.Errorf("%w: %s", ErrSkippedTest, strings.Join(pairs, ", ")) + } p, err := target.Deserialize(data, mode) if err != nil { - return nil, err + return nil, nil, err } if len(p.Calls) > prog.MaxCalls { - return nil, fmt.Errorf("longer than %d calls (%d)", prog.MaxCalls, len(p.Calls)) + return nil, nil, fmt.Errorf("longer than %d calls (%d)", prog.MaxCalls, len(p.Calls)) } // For some yet unknown reasons, programs with fail_nth > 0 may sneak in. Ignore them. for _, call := range p.Calls { if call.Props.FailNth > 0 { - return nil, fmt.Errorf("input has fail_nth > 0") + return nil, nil, fmt.Errorf("input has fail_nth > 0") } } - return p, nil + return p, properties, nil } type FilteredCandidates struct { diff --git a/pkg/manager/seeds_test.go b/pkg/manager/seeds_test.go new file mode 100644 index 000000000..d683c4a50 --- /dev/null +++ b/pkg/manager/seeds_test.go @@ -0,0 +1,29 @@ +// Copyright 2024 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +package manager + +import ( + "testing" +) + +func TestRequires(t *testing.T) { + { + requires := parseRequires([]byte("# requires: manual arch=amd64")) + if !checkArch(requires, "amd64") { + t.Fatalf("amd64 does not pass check") + } + if checkArch(requires, "riscv64") { + t.Fatalf("riscv64 passes check") + } + } + { + requires := parseRequires([]byte("# requires: -arch=arm64 manual -arch=riscv64")) + if !checkArch(requires, "amd64") { + t.Fatalf("amd64 does not pass check") + } + if checkArch(requires, "riscv64") { + t.Fatalf("riscv64 passes check") + } + } +} diff --git a/pkg/runtest/run.go b/pkg/runtest/run.go index 1ea5c7a7e..213ce3f58 100644 --- a/pkg/runtest/run.go +++ b/pkg/runtest/run.go @@ -14,6 +14,7 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "os" "path/filepath" @@ -305,13 +306,10 @@ func parseProg(target *prog.Target, dir, filename string, requires map[string]bo if err != nil { return nil, nil, nil, fmt.Errorf("failed to read %v: %w", filename, err) } - properties := parseRequires(data) - // Need to check arch requirement early as some programs - // may fail to deserialize on some arches due to missing syscalls. - if !checkArch(properties, target.Arch) || !match(properties, requires) { + p, properties, err := manager.ParseSeedWithRequirements(target, data, requires) + if errors.Is(err, manager.ErrSkippedTest) { return nil, nil, nil, nil } - p, err := manager.ParseSeedStrict(target, data) if err != nil { return nil, nil, nil, fmt.Errorf("failed to deserialize %v: %w", filename, err) } @@ -366,42 +364,11 @@ func parseProg(target *prog.Target, dir, filename string, requires map[string]bo return p, properties, info, nil } -func parseRequires(data []byte) map[string]bool { - requires := make(map[string]bool) - for s := bufio.NewScanner(bytes.NewReader(data)); s.Scan(); { - const prefix = "# requires:" - line := s.Text() - if !strings.HasPrefix(line, prefix) { - continue - } - for _, req := range strings.Fields(line[len(prefix):]) { - positive := true - if req[0] == '-' { - positive = false - req = req[1:] - } - requires[req] = positive - } - } - return requires -} - -func checkArch(requires map[string]bool, arch string) bool { - for req, positive := range requires { - const prefix = "arch=" - if strings.HasPrefix(req, prefix) && - arch != req[len(prefix):] == positive { - return false - } - } - return true -} - func (ctx *Context) produceTest(req *runRequest, name string, properties, requires map[string]bool, results *flatrpc.ProgInfo) { req.name = name req.results = results - if !match(properties, requires) { + if !manager.MatchRequirements(properties, requires) { req.skip = "excluded by constraints" } ctx.createTest(req) @@ -445,27 +412,6 @@ func (ctx *Context) submit(req *runRequest) { req.executor.Submit(req.Request) } -func match(props, requires map[string]bool) bool { - for req, positive := range requires { - if positive { - if !props[req] { - return false - } - continue - } - matched := true - for _, req1 := range strings.Split(req, ",") { - if !props[req1] { - matched = false - } - } - if matched { - return false - } - } - return true -} - func (ctx *Context) createSyzTest(p *prog.Prog, sandbox string, threaded, cov bool) (*runRequest, error) { var opts flatrpc.ExecOpts sandboxFlags, err := flatrpc.SandboxToFlags(sandbox) diff --git a/pkg/runtest/run_test.go b/pkg/runtest/run_test.go index 5d6235fd8..dd2c4a245 100644 --- a/pkg/runtest/run_test.go +++ b/pkg/runtest/run_test.go @@ -589,24 +589,3 @@ func TestParsing(t *testing.T) { } } } - -func TestRequires(t *testing.T) { - { - requires := parseRequires([]byte("# requires: manual arch=amd64")) - if !checkArch(requires, "amd64") { - t.Fatalf("amd64 does not pass check") - } - if checkArch(requires, "riscv64") { - t.Fatalf("riscv64 passes check") - } - } - { - requires := parseRequires([]byte("# requires: -arch=arm64 manual -arch=riscv64")) - if !checkArch(requires, "amd64") { - t.Fatalf("amd64 does not pass check") - } - if checkArch(requires, "riscv64") { - t.Fatalf("riscv64 passes check") - } - } -} -- cgit mrf-deployment