From: Katie Hockman Date: Wed, 13 Oct 2021 20:49:27 +0000 (-0400) Subject: internal/fuzz: fix bugs with minimization X-Git-Tag: go1.18beta1~883 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=cfe6763783615233ec7ae863784b898718d14c40;p=gostls13.git internal/fuzz: fix bugs with minimization This pulls in some code and tests from CL 353355. This change makes some refactors for when we read to and write from memory during minimization. That fixes a bug when minimizing interesting inputs. Now, if an error occurs while minimizing an interesting input, that value will continue to be minimized as a crash, and returned to the user. This change also allows minimization of a crash that occurred during the warmup phase. We don't want to minimize failures in the seed corpus, but if an entry in the cache causes a new failure, then there's no compelling reason why we shouldn't try to minimize it. Fixes #48731 Change-Id: I7262cecd8ea7ae6fdf932f3a36db55fb062a1f2a Reviewed-on: https://go-review.googlesource.com/c/go/+/355691 Trust: Katie Hockman Run-TryBot: Katie Hockman TryBot-Result: Go Bot Reviewed-by: Roland Shoemaker --- diff --git a/src/cmd/go/testdata/script/test_fuzz_minimize.txt b/src/cmd/go/testdata/script/test_fuzz_minimize.txt index 0a0359fabb..56abc68104 100644 --- a/src/cmd/go/testdata/script/test_fuzz_minimize.txt +++ b/src/cmd/go/testdata/script/test_fuzz_minimize.txt @@ -7,56 +7,72 @@ env GOCACHE=$WORK/gocache # Test that fuzzminimizetime cannot be negative seconds -! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1ms minimizer_test.go +! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1ms . ! stdout '^ok' ! stdout 'contains a non-zero byte' stdout 'invalid duration' stdout FAIL # Test that fuzzminimizetime cannot be negative times -! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1x minimizer_test.go +! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x -fuzzminimizetime=-1x . ! stdout '^ok' ! stdout 'contains a non-zero byte' stdout 'invalid count' stdout FAIL # Test that fuzzminimizetime can be zero seconds, and minimization is disabled -! go test -fuzz=FuzzMinimizeZeroDurationSet -run=FuzzMinimizeZeroDurationSet -fuzztime=10000x -fuzzminimizetime=0s minimizer_test.go +! go test -fuzz=FuzzMinimizeZeroDurationSet -run=FuzzMinimizeZeroDurationSet -fuzztime=10000x -fuzzminimizetime=0s . ! stdout '^ok' ! stdout 'minimizing' stdout 'there was an Error' stdout FAIL # Test that fuzzminimizetime can be zero times, and minimization is disabled -! go test -fuzz=FuzzMinimizeZeroLimitSet -run=FuzzMinimizeZeroLimitSet -fuzztime=10000x -fuzzminimizetime=0x minimizer_test.go +! go test -fuzz=FuzzMinimizeZeroLimitSet -run=FuzzMinimizeZeroLimitSet -fuzztime=10000x -fuzzminimizetime=0x . ! stdout '^ok' ! stdout 'minimizing' stdout 'there was an Error' stdout FAIL +# Test that minimization occurs for a crash that appears while minimizing a +# newly found interesting input. There must be only one worker for this test to +# be flaky like we want. +! go test -fuzz=FuzzMinimizerCrashInMinimization -run=FuzzMinimizerCrashInMinimization -fuzztime=10000x -parallel=1 . +! stdout '^ok' +stdout 'got the minimum size!' +stdout 'flaky failure' +stdout FAIL + +# Make sure the crash that was written will fail when run with go test +! go test -run=FuzzMinimizerCrashInMinimization . + +# Clear testdata. +rm testdata + # Test that minimization is working for recoverable errors. -! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x minimizer_test.go +! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=10000x . ! stdout '^ok' stdout 'got the minimum size!' -stdout 'contains a non-zero byte' +# The error message that was printed should be for the one written to testdata. +stdout 'contains a non-zero byte of length 50' stdout FAIL # Check that the bytes written to testdata are of length 50 (the minimum size) -go run check_testdata.go FuzzMinimizerRecoverable 50 +go run ./check_testdata FuzzMinimizerRecoverable 50 # Test that re-running the minimized value causes a crash. -! go test -run=FuzzMinimizerRecoverable minimizer_test.go +! go test -run=FuzzMinimizerRecoverable . rm testdata # Test that minimization doesn't run for non-recoverable errors. -! go test -fuzz=FuzzMinimizerNonrecoverable -run=FuzzMinimizerNonrecoverable -fuzztime=10000x minimizer_test.go +! go test -fuzz=FuzzMinimizerNonrecoverable -run=FuzzMinimizerNonrecoverable -fuzztime=10000x . ! stdout '^ok' ! stdout 'minimizing' stdout 'fuzzing process terminated unexpectedly: exit status 99' stdout FAIL # Check that re-running the value causes a crash. -! go test -run=FuzzMinimizerNonrecoverable minimizer_test.go +! go test -run=FuzzMinimizerNonrecoverable . rm testdata # Clear the fuzzing cache. There may already be minimized inputs that would @@ -65,35 +81,43 @@ go clean -fuzzcache # Test that minimization can be cancelled by fuzzminimizetime and the latest # crash will still be logged and written to testdata. -! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=100x -fuzzminimizetime=1x minimizer_test.go +! go test -fuzz=FuzzMinimizerRecoverable -run=FuzzMinimizerRecoverable -fuzztime=100x -fuzzminimizetime=1x . ! stdout '^ok' stdout 'testdata[/\\]fuzz[/\\]FuzzMinimizerRecoverable[/\\]' ! stdout 'got the minimum size!' # it shouldn't have had enough time to minimize it stdout FAIL # Test that re-running the unminimized value causes a crash. -! go test -run=FuzzMinimizerRecoverable minimizer_test.go - -! go test -fuzz=FuzzMinimizerTooSlow -run=FuzzMinimizerTooSlow -fuzzminimizetime=3s minimizer_test.go -stdout 'fuzz: minimizing' -stdout 'fuzz: elapsed: \d+s, minimizing' -stdout 'testdata[/\\]fuzz[/\\]FuzzMinimizerTooSlow[/\\]' -stdout FAIL +! go test -run=FuzzMinimizerRecoverable . # TODO(jayconrod,katiehockman): add a test which verifies that the right bytes # are written to testdata in the case of an interrupt during minimization. -- go.mod -- -module m +module example.com/y go 1.16 --- minimizer_test.go -- -package fuzz_test +-- y.go -- +package y import ( + "bytes" + "io" +) + +func Y(w io.Writer, b []byte) { + if !bytes.Equal(b, []byte("y")) { + w.Write([]byte("not equal")) + } +} +-- y_test.go -- +package y + +import ( + "bytes" + "io" "os" "testing" - "time" ) func FuzzMinimizeZeroDurationSet(f *testing.F) { @@ -126,42 +150,42 @@ func FuzzMinimizerRecoverable(f *testing.F) { if len(b) == 50 { t.Log("got the minimum size!") } - t.Fatal("contains a non-zero byte") + t.Fatalf("contains a non-zero byte of length %d", len(b)) } } }) } func FuzzMinimizerNonrecoverable(f *testing.F) { - f.Add(make([]byte, 100)) f.Fuzz(func(t *testing.T, b []byte) { - if len(b) < 50 { - // Make sure that b is large enough that it can be minimized - return - } - // Given the randomness of the mutations, this should allow the - // minimizer to trim down the value a bit. - for _, n := range b { - if n != 0 { - t.Log("contains a non-zero byte") - os.Exit(99) - } - } + os.Exit(99) }) } -func FuzzMinimizerTooSlow(f *testing.F) { +func FuzzMinimizerCrashInMinimization(f *testing.F) { + seed := make([]byte, 1000) + f.Add(seed) f.Fuzz(func(t *testing.T, b []byte) { - if len(b) > 50 { - t.Error("error here") - time.Sleep(2 * time.Second) + if len(b) < 50 || len(b) > 1100 { + // Make sure that b is large enough that it can be minimized + return + } + if !bytes.Equal(b, seed) { + // This should have hit a new edge, and the interesting input + // should be attempting minimization + Y(io.Discard, b) + } + if len(b) < 350 { + t.Error("flaky failure") + } + if len(b) == 50 { + t.Log("got the minimum size!") } }) } - --- check_testdata.go -- -// +build ignore - +-- empty/empty.go -- +package empty +-- check_testdata/check_testdata.go -- package main import ( diff --git a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt b/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt index 5d04d8c022..18f634a3b6 100644 --- a/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt +++ b/src/cmd/go/testdata/script/test_fuzz_seed_corpus.txt @@ -47,6 +47,23 @@ cp cache-file $GOCACHE/fuzz/example.com/x/FuzzWithCache/1 stdout 'Crash written to testdata[/\\]fuzz[/\\]FuzzWithCache[/\\]' stdout FAIL +# Write a crashing input to the cache +mkdir $GOCACHE/fuzz/example.com/x/FuzzWithMinimizableCache +cp cache-file-bytes $GOCACHE/fuzz/example.com/x/FuzzWithMinimizableCache/1 + +# Test that fuzzing a target with a failure in the cache minimizes it and writes +# the new crash to testdata/fuzz +! go test -fuzz=FuzzWithMinimizableCache -run=FuzzWithMinimizableCache -fuzztime=10000x +! stdout ^ok +stdout 'gathering baseline coverage' +stdout 'got the minimum size!' +stdout 'contains a non-zero byte of length 10' +stdout 'Crash written to testdata[/\\]fuzz[/\\]FuzzWithMinimizableCache[/\\]' +stdout FAIL +# Make sure this crash didn't come from fuzzing +# (the log line that states fuzzing began shouldn't have printed) +! stdout 'execs' + # Clear the fuzz cache and make sure it's gone go clean -fuzzcache ! exists $GOCACHE/fuzz @@ -158,6 +175,22 @@ func FuzzWithCache(f *testing.F) { }) } +func FuzzWithMinimizableCache(f *testing.F) { + f.Fuzz(func(t *testing.T, b []byte) { + if len(b) < 10 { + return + } + for _, n := range b { + if n != 0 { + if len(b) == 10 { + t.Log("got the minimum size!") + } + t.Fatalf("contains a non-zero byte of length %d", len(b)) + } + } + }) +} + func FuzzRunNoneWithCache(f *testing.F) { f.Fuzz(func(t *testing.T, i int) { if i == 10 { @@ -170,4 +203,7 @@ go test fuzz v1 int(10) -- cache-file -- go test fuzz v1 -int(10) \ No newline at end of file +int(10) +-- cache-file-bytes -- +go test fuzz v1 +[]byte("11111111111111111111") \ No newline at end of file diff --git a/src/internal/fuzz/fuzz.go b/src/internal/fuzz/fuzz.go index 03071d5521..5b3819be75 100644 --- a/src/internal/fuzz/fuzz.go +++ b/src/internal/fuzz/fuzz.go @@ -875,12 +875,10 @@ func (c *coordinator) updateCoverage(newCoverage []byte) int { } // canMinimize returns whether the coordinator should attempt to find smaller -// inputs that reproduce a crash or new coverage. It shouldn't do this if it -// is in the warmup phase. +// inputs that reproduce a crash or new coverage. func (c *coordinator) canMinimize() bool { return c.minimizationAllowed && - (c.opts.Limit == 0 || c.count+c.countWaiting < c.opts.Limit) && - !c.warmupRun() + (c.opts.Limit == 0 || c.count+c.countWaiting < c.opts.Limit) } func (c *coordinator) elapsed() time.Duration { diff --git a/src/internal/fuzz/minimize_test.go b/src/internal/fuzz/minimize_test.go index 410b78310b..dd76baff51 100644 --- a/src/internal/fuzz/minimize_test.go +++ b/src/internal/fuzz/minimize_test.go @@ -262,13 +262,12 @@ func TestMinimizeInput(t *testing.T) { } } -// TestMinimizeInputCoverageError checks that if we're minimizing an interesting -// input (one that we don't expect to cause an error), and the fuzz function -// returns an error, minimizing fails, and we return the error quickly. -func TestMinimizeInputCoverageError(t *testing.T) { - errOhNo := errors.New("ohno") +// TestMinimizeFlaky checks that if we're minimizing an interesting +// input and a flaky failure occurs, that minimization was not indicated +// to be successful, and the error isn't returned (since it's flaky). +func TestMinimizeFlaky(t *testing.T) { ws := &workerServer{fuzzFn: func(e CorpusEntry) error { - return errOhNo + return errors.New("ohno") }} keepCoverage := make([]byte, len(coverageSnapshot)) count := int64(0) @@ -277,7 +276,7 @@ func TestMinimizeInputCoverageError(t *testing.T) { if success { t.Error("unexpected success") } - if err != errOhNo { + if err != nil { t.Errorf("unexpected error: %v", err) } if count != 1 { diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go index 0c428ed832..e3827b112a 100644 --- a/src/internal/fuzz/worker.go +++ b/src/internal/fuzz/worker.go @@ -261,8 +261,8 @@ func (w *worker) minimize(ctx context.Context, input fuzzMinimizeInput) (min fuz return fuzzResult{}, fmt.Errorf("fuzzing process terminated unexpectedly while minimizing: %w", w.waitErr) } - if input.crasherMsg != "" && resp.Err == "" && !resp.Success { - return fuzzResult{}, fmt.Errorf("attempted to minimize but could not reproduce") + if input.crasherMsg != "" && resp.Err == "" { + return fuzzResult{}, fmt.Errorf("attempted to minimize a crash but could not reproduce") } return fuzzResult{ @@ -509,12 +509,11 @@ type minimizeArgs struct { // minimizeResponse contains results from workerServer.minimize. type minimizeResponse struct { - // Success is true if the worker found a smaller input, stored in shared - // memory, that was "interesting" for the same reason as the original input. - // If minimizeArgs.KeepCoverage was set, the minimized input preserved at - // least one coverage bit and did not cause an error. Otherwise, the - // minimized input caused some error, recorded in Err. - Success bool + // WroteToMem is true if the worker found a smaller input and wrote it to + // shared memory. If minimizeArgs.KeepCoverage was set, the minimized input + // preserved at least one coverage bit and did not cause an error. + // Otherwise, the minimized input caused some error, recorded in Err. + WroteToMem bool // Err is the error string caused by the value in shared memory, if any. Err string @@ -777,32 +776,31 @@ func (ws *workerServer) minimize(ctx context.Context, args minimizeArgs) (resp m } // Minimize the values in vals, then write to shared memory. We only write - // to shared memory after completing minimization. If the worker terminates - // unexpectedly before then, the coordinator will use the original input. - resp.Success, err = ws.minimizeInput(ctx, vals, &mem.header().count, args.Limit, args.KeepCoverage) - if resp.Success { + // to shared memory after completing minimization. + // TODO(48165): If the worker terminates unexpectedly during minimization, + // the coordinator has no way of retrieving the crashing input. + success, err := ws.minimizeInput(ctx, vals, &mem.header().count, args.Limit, args.KeepCoverage) + if success { writeToMem(vals, mem) - } - if err != nil { - resp.Err = err.Error() - } else if resp.Success { - resp.CoverageData = coverageSnapshot + resp.WroteToMem = true + if err != nil { + resp.Err = err.Error() + } else { + resp.CoverageData = coverageSnapshot + } } return resp } // minimizeInput applies a series of minimizing transformations on the provided -// vals, ensuring that each minimization still causes an error in fuzzFn. Before -// every call to fuzzFn, it marshals the new vals and writes it to the provided -// mem just in case an unrecoverable error occurs. It uses the context to -// determine how long to run, stopping once closed. It returns a bool -// indicating whether minimization was successful and an error if one was found. +// vals, ensuring that each minimization still causes an error in fuzzFn. It +// uses the context to determine how long to run, stopping once closed. It +// returns a bool indicating whether minimization was successful and an error if +// one was found. func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, count *int64, limit int64, keepCoverage []byte) (success bool, retErr error) { - wantError := keepCoverage == nil shouldStop := func() bool { return ctx.Err() != nil || - (limit > 0 && *count >= limit) || - (retErr != nil && !wantError) + (limit > 0 && *count >= limit) } if shouldStop() { return false, nil @@ -812,11 +810,12 @@ func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, c // If not, then whatever caused us to think the value was interesting may // have been a flake, and we can't minimize it. *count++ - if retErr = ws.fuzzFn(CorpusEntry{Values: vals}); retErr == nil && wantError { - return false, nil - } else if retErr != nil && !wantError { - return false, retErr - } else if keepCoverage != nil && !hasCoverageBit(keepCoverage, coverageSnapshot) { + retErr = ws.fuzzFn(CorpusEntry{Values: vals}) + if keepCoverage != nil { + if !hasCoverageBit(keepCoverage, coverageSnapshot) || retErr != nil { + return false, nil + } + } else if retErr == nil { return false, nil } @@ -881,7 +880,13 @@ func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, c err := ws.fuzzFn(CorpusEntry{Values: vals}) if err != nil { retErr = err - return wantError + if keepCoverage != nil { + // Now that we've found a crash, that's more important than any + // minimization of interesting inputs that was being done. Clear out + // keepCoverage to only minimize the crash going forward. + keepCoverage = nil + } + return true } if keepCoverage != nil && hasCoverageBit(keepCoverage, coverageSnapshot) { return true @@ -939,7 +944,7 @@ func (ws *workerServer) minimizeInput(ctx context.Context, vals []interface{}, c panic("unreachable") } } - return (wantError || retErr == nil), retErr + return true, retErr } func writeToMem(vals []interface{}, mem *sharedMem) { @@ -1024,7 +1029,7 @@ func (wc *workerClient) minimize(ctx context.Context, entryIn CorpusEntry, args } defer func() { wc.memMu <- mem }() resp.Count = mem.header().count - if resp.Success { + if resp.WroteToMem { entryOut.Data = mem.valueCopy() entryOut.Values, err = unmarshalCorpusFile(entryOut.Data) h := sha256.Sum256(entryOut.Data)