From 88f59346334b1ec60ea3cc78af174856aee31866 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Mon, 24 Dec 2018 09:41:43 +0100 Subject: [PATCH] vm: allow fine-grained control over program exit conditions Currently we only support canExit flag. However there are actually 3 separate conditions: - program can exit normally - program can timeout (e.g. fuzzer test or runtest can't) - program can exit with error (e.g. C test can) Allow to specify these 3 conditions separately. --- pkg/instance/instance.go | 7 +++--- pkg/repro/repro.go | 3 ++- syz-manager/manager.go | 2 +- tools/syz-crush/crush.go | 2 +- tools/syz-runtest/runtest.go | 2 +- vm/vm.go | 45 ++++++++++++++++++++++++---------- vm/vm_test.go | 47 ++++++++++++++++++++++++------------ 7 files changed, 73 insertions(+), 35 deletions(-) diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index 1d35efa75178..64d135429d7b 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -265,11 +265,11 @@ func (inst *inst) testInstance() error { cmd := FuzzerCmd(fuzzerBin, executorBin, "test", inst.cfg.TargetOS, inst.cfg.TargetArch, fwdAddr, inst.cfg.Sandbox, 0, 0, false, false, true, false) - outc, errc, err := inst.vm.Run(5*time.Minute, nil, cmd) + outc, errc, err := inst.vm.Run(10*time.Minute, nil, cmd) if err != nil { return fmt.Errorf("failed to run binary in VM: %v", err) } - rep := inst.vm.MonitorExecution(outc, errc, inst.reporter, true) + rep := inst.vm.MonitorExecution(outc, errc, inst.reporter, vm.ExitNormal) if rep != nil { if err := inst.reporter.Symbolize(rep); err != nil { // TODO(dvyukov): send such errors to dashboard. @@ -350,7 +350,8 @@ func (inst *inst) testProgram(command string, testTime time.Duration) error { if err != nil { return fmt.Errorf("failed to run binary in VM: %v", err) } - rep := inst.vm.MonitorExecution(outc, errc, inst.reporter, true) + rep := inst.vm.MonitorExecution(outc, errc, inst.reporter, + vm.ExitTimeout|vm.ExitNormal|vm.ExitError) if rep == nil { return nil } diff --git a/pkg/repro/repro.go b/pkg/repro/repro.go index a10e02f27e3c..1fdffbc4d8d3 100644 --- a/pkg/repro/repro.go +++ b/pkg/repro/repro.go @@ -587,7 +587,8 @@ func (ctx *context) testImpl(inst *vm.Instance, command string, duration time.Du if err != nil { return false, fmt.Errorf("failed to run command in VM: %v", err) } - rep := inst.MonitorExecution(outc, errc, ctx.reporter, true) + rep := inst.MonitorExecution(outc, errc, ctx.reporter, + vm.ExitTimeout|vm.ExitNormal|vm.ExitError) if rep == nil { ctx.reproLog(2, "program did not crash") return false, nil diff --git a/syz-manager/manager.go b/syz-manager/manager.go index 93e518611a52..37f436c2989d 100644 --- a/syz-manager/manager.go +++ b/syz-manager/manager.go @@ -574,7 +574,7 @@ func (mgr *Manager) runInstance(index int) (*Crash, error) { return nil, fmt.Errorf("failed to run fuzzer: %v", err) } - rep := inst.MonitorExecution(outc, errc, mgr.reporter, false) + rep := inst.MonitorExecution(outc, errc, mgr.reporter, vm.ExitTimeout) if rep == nil { // This is the only "OK" outcome. log.Logf(0, "vm-%v: running for %v, restarting", index, time.Since(start)) diff --git a/tools/syz-crush/crush.go b/tools/syz-crush/crush.go index 3d184d5a44c0..1cc850c7ad3f 100644 --- a/tools/syz-crush/crush.go +++ b/tools/syz-crush/crush.go @@ -107,7 +107,7 @@ func runInstance(cfg *mgrconfig.Config, reporter report.Reporter, vmPool *vm.Poo } log.Logf(0, "vm-%v: crushing...", index) - rep := inst.MonitorExecution(outc, errc, reporter, false) + rep := inst.MonitorExecution(outc, errc, reporter, vm.ExitTimeout) if rep == nil { // This is the only "OK" outcome. log.Logf(0, "vm-%v: running long enough, restarting", index) diff --git a/tools/syz-runtest/runtest.go b/tools/syz-runtest/runtest.go index a84bc005d42d..b218522992b3 100644 --- a/tools/syz-runtest/runtest.go +++ b/tools/syz-runtest/runtest.go @@ -177,7 +177,7 @@ func (mgr *Manager) boot(name string, index int) (*report.Report, error) { if err != nil { return nil, fmt.Errorf("failed to run fuzzer: %v", err) } - rep := inst.MonitorExecution(outc, errc, mgr.reporter, true) + rep := inst.MonitorExecution(outc, errc, mgr.reporter, vm.ExitNormal) return rep, nil } diff --git a/vm/vm.go b/vm/vm.go index 00fb3f33b70c..fcae3a0c966e 100644 --- a/vm/vm.go +++ b/vm/vm.go @@ -135,19 +135,30 @@ func (inst *Instance) Close() { os.RemoveAll(inst.workdir) } +type ExitCondition int + +const ( + // The program is allowed to exit after timeout. + ExitTimeout = ExitCondition(1 << iota) + // The program is allowed to exit with no errors. + ExitNormal + // The program is allowed to exit with errors. + ExitError +) + // MonitorExecution monitors execution of a program running inside of a VM. // It detects kernel oopses in output, lost connections, hangs, etc. // outc/errc is what vm.Instance.Run returns, reporter parses kernel output for oopses. -// If canExit is false and the program exits, it is treated as an error. +// Exit says which exit modes should be considered as errors/OK. // Returns a non-symbolized crash report, or nil if no error happens. func (inst *Instance) MonitorExecution(outc <-chan []byte, errc <-chan error, - reporter report.Reporter, canExit bool) (rep *report.Report) { + reporter report.Reporter, exit ExitCondition) (rep *report.Report) { mon := &monitor{ inst: inst, outc: outc, errc: errc, reporter: reporter, - canExit: canExit, + exit: exit, } lastExecuteTime := time.Now() ticker := time.NewTicker(tickerPeriod) @@ -159,13 +170,24 @@ func (inst *Instance) MonitorExecution(outc <-chan []byte, errc <-chan error, case nil: // The program has exited without errors, // but wait for kernel output in case there is some delayed oops. - return mon.extractError("") + crash := "" + if mon.exit&ExitNormal == 0 { + crash = lostConnectionCrash + } + return mon.extractError(crash) case ErrTimeout: + if mon.exit&ExitTimeout == 0 { + return mon.extractError(timeoutCrash) + } return nil default: // Note: connection lost can race with a kernel oops message. // In such case we want to return the kernel oops. - return mon.extractError(lostConnectionCrash) + crash := "" + if mon.exit&ExitError == 0 { + crash = lostConnectionCrash + } + return mon.extractError(crash) } case out, ok := <-outc: if !ok { @@ -229,14 +251,13 @@ type monitor struct { outc <-chan []byte errc <-chan error reporter report.Reporter - canExit bool + exit ExitCondition output []byte matchPos int } func (mon *monitor) extractError(defaultError string) *report.Report { - crashed := defaultError != "" || !mon.canExit - if crashed { + if defaultError != "" { // N.B. we always wait below for other errors. diag, _ := mon.inst.Diagnose() if len(diag) > 0 { @@ -251,10 +272,7 @@ func (mon *monitor) extractError(defaultError string) *report.Report { } if !mon.reporter.ContainsCrash(mon.output[mon.matchPos:]) { if defaultError == "" { - if mon.canExit { - return nil - } - defaultError = lostConnectionCrash + return nil } rep := &report.Report{ Title: defaultError, @@ -263,7 +281,7 @@ func (mon *monitor) extractError(defaultError string) *report.Report { } return rep } - if !crashed { + if defaultError == "" { diag, wait := mon.inst.Diagnose() if len(diag) > 0 { mon.output = append(mon.output, "DIAGNOSIS:\n"...) @@ -314,6 +332,7 @@ const ( lostConnectionCrash = "lost connection to test machine" noOutputCrash = "no output from test machine" + timeoutCrash = "timed out" executingProgramStr1 = "executing program" // syz-fuzzer output executingProgramStr2 = "executed programs:" // syz-execprog output fuzzerPreemptedStr = "SYZ-FUZZER: PREEMPTED" diff --git a/vm/vm_test.go b/vm/vm_test.go index ac86ad9c50c6..702e2626e157 100644 --- a/vm/vm_test.go +++ b/vm/vm_test.go @@ -83,7 +83,7 @@ func init() { type Test struct { Name string - CanExit bool // if the program is allowed to exit normally + Exit ExitCondition DiagnoseBug bool // Diagnose produces output that is detected as kernel crash DiagnoseNoWait bool // Diagnose returns output directly rather than to console Body func(outc chan []byte, errc chan error) @@ -92,8 +92,8 @@ type Test struct { var tests = []*Test{ { - Name: "program-exits-normally", - CanExit: true, + Name: "program-exits-normally", + Exit: ExitNormal, Body: func(outc chan []byte, errc chan error) { time.Sleep(time.Second) errc <- nil @@ -111,7 +111,7 @@ var tests = []*Test{ }, { Name: "#875-diagnose-bugs", - CanExit: true, + Exit: ExitNormal, DiagnoseBug: true, Body: func(outc chan []byte, errc chan error) { errc <- nil @@ -166,8 +166,8 @@ var tests = []*Test{ }, }, { - Name: "program-exits-but-kernel-crashes-afterwards", - CanExit: true, + Name: "program-exits-but-kernel-crashes-afterwards", + Exit: ExitNormal, Body: func(outc chan []byte, errc chan error) { errc <- nil time.Sleep(time.Second) @@ -183,10 +183,20 @@ var tests = []*Test{ }, { Name: "timeout", + Exit: ExitTimeout, Body: func(outc chan []byte, errc chan error) { errc <- vmimpl.ErrTimeout }, }, + { + Name: "bad-timeout", + Body: func(outc chan []byte, errc chan error) { + errc <- vmimpl.ErrTimeout + }, + Report: &report.Report{ + Title: timeoutCrash, + }, + }, { Name: "program-crashes", Body: func(outc chan []byte, errc chan error) { @@ -196,6 +206,13 @@ var tests = []*Test{ Title: lostConnectionCrash, }, }, + { + Name: "program-crashes-expected", + Exit: ExitError, + Body: func(outc chan []byte, errc chan error) { + errc <- fmt.Errorf("error") + }, + }, { Name: "no-output-1", Body: func(outc chan []byte, errc chan error) { @@ -217,8 +234,8 @@ var tests = []*Test{ }, }, { - Name: "no-no-output-1", - CanExit: true, + Name: "no-no-output-1", + Exit: ExitNormal, Body: func(outc chan []byte, errc chan error) { for i := 0; i < 5; i++ { time.Sleep(time.Second) @@ -228,8 +245,8 @@ var tests = []*Test{ }, }, { - Name: "no-no-output-2", - CanExit: true, + Name: "no-no-output-2", + Exit: ExitNormal, Body: func(outc chan []byte, errc chan error) { for i := 0; i < 5; i++ { time.Sleep(time.Second) @@ -239,8 +256,8 @@ var tests = []*Test{ }, }, { - Name: "outc-closed", - CanExit: true, + Name: "outc-closed", + Exit: ExitTimeout, Body: func(outc chan []byte, errc chan error) { close(outc) time.Sleep(time.Second) @@ -248,8 +265,8 @@ var tests = []*Test{ }, }, { - Name: "lots-of-output", - CanExit: true, + Name: "lots-of-output", + Exit: ExitTimeout, Body: func(outc chan []byte, errc chan error) { for i := 0; i < 100; i++ { outc <- []byte("something\n") @@ -308,7 +325,7 @@ func testMonitorExecution(t *testing.T, test *Test) { test.Body(testInst.outc, testInst.errc) done <- true }() - rep := inst.MonitorExecution(outc, errc, reporter, test.CanExit) + rep := inst.MonitorExecution(outc, errc, reporter, test.Exit) <-done if test.Report != nil && rep == nil { t.Fatalf("got no report")