pkg/instance: use execprog to do basic instance testing

When we accept new kernels for fuzzing we need more extensive testing,
but syz-ci switched to using syz-manager for this purpose.
Now instance testing is used only for bisection and patch testing,
which does not need such extensive image testing (it may even harm).
So just run a simple program as a testing.
It also uses the same features as the target reproducer,
so e.g. if the reproducer does not use wifi, we won't test it,
which reduces changes of unrelated kernel bugs.

3 files changed, 49 insertions, 146 deletions

diff --git a/pkg/instance/execprog.go b/pkg/instance/execprog.go
index 4cc8dd1f5..6abf8fd9c 100644
--- a/pkg/instance/execprog.go
+++ b/pkg/instance/execprog.go
@@ -43,7 +43,7 @@ type RunResult struct {
 const (
 	// It's reasonable to expect that tools/syz-execprog should not normally
 	// return a non-zero exit code.
-	syzExitConditions = vm.ExitTimeout | vm.ExitNormal
+	SyzExitConditions = vm.ExitTimeout | vm.ExitNormal
 	binExitConditions = vm.ExitTimeout | vm.ExitNormal | vm.ExitError
 )
 
@@ -161,7 +161,7 @@ func (inst *ExecProgInstance) RunCProgRaw(src []byte, target *prog.Target,
 }
 
 func (inst *ExecProgInstance) RunSyzProgFile(progFile string, duration time.Duration,
-	opts csource.Options) (*RunResult, error) {
+	opts csource.Options, exitCondition vm.ExitCondition) (*RunResult, error) {
 	vmProgFile, err := inst.VMInstance.Copy(progFile)
 	if err != nil {
 		return nil, &TestError{Title: fmt.Sprintf("failed to copy prog to VM: %v", err)}
@@ -174,17 +174,17 @@ func (inst *ExecProgInstance) RunSyzProgFile(progFile string, duration time.Dura
 	command := ExecprogCmd(inst.execprogBin, inst.executorBin, target.OS, target.Arch, opts.Sandbox,
 		opts.SandboxArg, opts.Repeat, opts.Threaded, opts.Collide, opts.Procs, faultCall, opts.FaultNth,
 		!inst.OldFlagsCompatMode, inst.mgrCfg.Timeouts.Slowdown, vmProgFile)
-	return inst.runCommand(command, duration, syzExitConditions)
+	return inst.runCommand(command, duration, exitCondition)
 }
 
 func (inst *ExecProgInstance) RunSyzProg(syzProg []byte, duration time.Duration,
-	opts csource.Options) (*RunResult, error) {
+	opts csource.Options, exitCondition vm.ExitCondition) (*RunResult, error) {
 	progFile, err := osutil.WriteTempFile(syzProg)
 	if err != nil {
 		return nil, err
 	}
 	defer os.Remove(progFile)
-	return inst.RunSyzProgFile(progFile, duration, opts)
+	return inst.RunSyzProgFile(progFile, duration, opts, exitCondition)
 }
 
 func (inst *ExecProgInstance) Close() {
diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go
index 4e5f95e2d..6f018422b 100644
--- a/pkg/instance/instance.go
+++ b/pkg/instance/instance.go
@@ -9,7 +9,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"net"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -349,70 +348,38 @@ func (inst *inst) test() EnvTestResult {
 	return ret
 }
 
-// testInstance tests basic operation of the provided VM
-// (that we can copy binaries, run binaries, they can connect to host, run syzkaller programs, etc).
+// testInstance tests that the VM does not crash on a simple program.
 // TestError is returned if there is a problem with the kernel (e.g. crash).
 func (inst *inst) testInstance() error {
-	ln, err := net.Listen("tcp", ":")
-	if err != nil {
-		return fmt.Errorf("failed to open listening socket: %w", err)
-	}
-	defer ln.Close()
-	acceptErr := make(chan error, 1)
-	go func() {
-		conn, err := ln.Accept()
-		if err == nil {
-			conn.Close()
-		}
-		acceptErr <- err
-	}()
-	fwdAddr, err := inst.vm.Forward(ln.Addr().(*net.TCPAddr).Port)
+	execProg, err := SetupExecProg(inst.vm, inst.cfg, inst.reporter, &OptionalConfig{
+		OldFlagsCompatMode: !inst.optionalFlags,
+	})
 	if err != nil {
-		return fmt.Errorf("failed to setup port forwarding: %w", err)
+		return err
 	}
-
-	fuzzerBin, err := inst.vm.Copy(inst.cfg.FuzzerBin)
+	// Note: we create the test program on a newer syzkaller revision and pass it to the old execprog.
+	// We rely on the non-strict program parsing to parse it successfuly.
+	testProg := inst.cfg.Target.DataMmapProg().Serialize()
+	// Use the same options as the target reproducer.
+	// E.g. if it does not use wifi, we won't test it, which reduces changes of unrelated kernel bugs.
+	// Note: we keep fault injection if it's enabled in the reproducer to test that fault injection works
+	// (does not produce some kernel oops when activated).
+	opts, err := inst.csourceOptions()
 	if err != nil {
-		return &TestError{Title: fmt.Sprintf("failed to copy test binary to VM: %v", err)}
-	}
-
-	// If ExecutorBin is provided, it means that syz-executor is already in the image,
-	// so no need to copy it.
-	executorBin := inst.cfg.SysTarget.ExecutorBin
-	if executorBin == "" {
-		executorBin, err = inst.vm.Copy(inst.cfg.ExecutorBin)
-		if err != nil {
-			return &TestError{Title: fmt.Sprintf("failed to copy test binary to VM: %v", err)}
-		}
+		return err
 	}
-
-	cmd := OldFuzzerCmd(fuzzerBin, executorBin, targets.TestOS, inst.cfg.TargetOS, inst.cfg.TargetArch, fwdAddr,
-		inst.cfg.Sandbox, inst.cfg.SandboxArg, 0, inst.cfg.Cover, true, inst.optionalFlags, inst.cfg.Timeouts.Slowdown)
-	timeout := 10 * time.Minute * inst.cfg.Timeouts.Scale
-	_, rep, err := inst.vm.Run(timeout, inst.reporter, cmd)
+	opts.Repeat = false
+	out, err := execProg.RunSyzProg(testProg, inst.cfg.Timeouts.NoOutputRunningTime, opts, vm.ExitNormal)
 	if err != nil {
-		return fmt.Errorf("failed to run binary in VM: %w", err)
+		return &TestError{Title: err.Error()}
 	}
-	if rep != nil {
-		if err := inst.reporter.Symbolize(rep); err != nil {
-			// TODO(dvyukov): send such errors to dashboard.
-			log.Logf(0, "failed to symbolize report: %v", err)
-		}
-		return &TestError{
-			Title:  rep.Title,
-			Report: rep,
-		}
-	}
-	select {
-	case err := <-acceptErr:
-		return err
-	case <-time.After(10 * time.Second):
-		return fmt.Errorf("test machine failed to connect to host")
+	if out.Report != nil {
+		return &TestError{Title: out.Report.Title, Report: out.Report}
 	}
+	return nil
 }
 
 func (inst *inst) testRepro() ([]byte, error) {
-	var err error
 	execProg, err := SetupExecProg(inst.vm, inst.cfg, inst.reporter, &OptionalConfig{
 		OldFlagsCompatMode: !inst.optionalFlags,
 	})
@@ -430,21 +397,12 @@ func (inst *inst) testRepro() ([]byte, error) {
 	}
 	out := []byte{}
 	if len(inst.reproSyz) > 0 {
-		var opts csource.Options
-		opts, err = csource.DeserializeOptions(inst.reproOpts)
+		opts, err := inst.csourceOptions()
 		if err != nil {
 			return nil, err
 		}
-		// Combine repro options and default options in a way that increases chances to reproduce the crash.
-		// First, we always enable threaded/collide as it should be [almost] strictly better.
-		// Executor does not support empty sandbox, so we use none instead.
-		// Finally, always use repeat and multiple procs.
-		if opts.Sandbox == "" {
-			opts.Sandbox = "none"
-		}
-		opts.Repeat, opts.Threaded = true, true
 		out, err = transformError(execProg.RunSyzProg(inst.reproSyz,
-			inst.cfg.Timeouts.NoOutputRunningTime, opts))
+			inst.cfg.Timeouts.NoOutputRunningTime, opts, SyzExitConditions))
 	}
 	if err == nil && len(inst.reproC) > 0 {
 		// We should test for more than full "no output" timeout, but the problem is that C reproducers
@@ -455,6 +413,28 @@ func (inst *inst) testRepro() ([]byte, error) {
 	return out, err
 }
 
+func (inst *inst) csourceOptions() (csource.Options, error) {
+	if len(inst.reproSyz) == 0 {
+		// This function is expected to be used only when we have a syz reproducer:
+		// either during syz reproducer testing, or during image testing
+		// for bisection or patch testing.
+		panic("no syz reproducer")
+	}
+	opts, err := csource.DeserializeOptions(inst.reproOpts)
+	if err != nil {
+		return opts, err
+	}
+	// Combine repro options and default options in a way that increases chances to reproduce the crash.
+	// First, we always enable threaded/collide as it should be [almost] strictly better.
+	// Executor does not support empty sandbox, so we use none instead.
+	// Finally, always use repeat and multiple procs.
+	if opts.Sandbox == "" {
+		opts.Sandbox = "none"
+	}
+	opts.Repeat, opts.Threaded = true, true
+	return opts, nil
+}
+
 type OptionalFuzzerArgs struct {
 	Slowdown   int
 	SandboxArg int64
@@ -504,18 +484,6 @@ func FuzzerCmd(args *FuzzerCmdArgs) string {
 		args.Procs, args.Cover, args.Debug, args.Test, verbosityArg, optionalArg)
 }
 
-func OldFuzzerCmd(fuzzer, executor, name, OS, arch, fwdAddr, sandbox string, sandboxArg int64, procs int,
-	cover, test, optionalFlags bool, slowdown int) string {
-	var optional *OptionalFuzzerArgs
-	if optionalFlags {
-		optional = &OptionalFuzzerArgs{Slowdown: slowdown, SandboxArg: sandboxArg}
-	}
-	return FuzzerCmd(&FuzzerCmdArgs{Fuzzer: fuzzer, Executor: executor, Name: name,
-		OS: OS, Arch: arch, FwdAddr: fwdAddr, Sandbox: sandbox,
-		Procs: procs, Verbosity: 0, Cover: cover, Debug: false, Test: test,
-		Optional: optional})
-}
-
 func ExecprogCmd(execprog, executor, OS, arch, sandbox string, sandboxArg int, repeat, threaded, collide bool,
 	procs, faultCall, faultNth int, optionalFlags bool, slowdown int, progFile string) string {
 	repeatCount := 1
diff --git a/pkg/instance/instance_test.go b/pkg/instance/instance_test.go
index 843334ebd..283921224 100644
--- a/pkg/instance/instance_test.go
+++ b/pkg/instance/instance_test.go
@@ -14,71 +14,6 @@ import (
 	"github.com/google/syzkaller/sys/targets"
 )
 
-func TestFuzzerCmd(t *testing.T) {
-	// IMPORTANT: if this test fails, do not fix it by changing flags here!
-	// Test how an old version of syz-fuzzer parses flags generated by the current FuzzerCmd.
-	// This actually happens in syz-ci when we test a patch for an old bug and use an old syz-fuzzer/execprog.
-	flags := flag.NewFlagSet("", flag.ContinueOnError)
-	flagName := flags.String("name", "", "unique name for manager")
-	flagArch := flags.String("arch", "", "target arch")
-	flagManager := flags.String("manager", "", "manager rpc address")
-	flagProcs := flags.Int("procs", 1, "number of parallel test processes")
-	flagLeak := flags.Bool("leak", false, "detect memory leaks")
-	flagOutput := flags.String("output", "stdout", "write programs to none/stdout/dmesg/file")
-	flagPprof := flags.String("pprof", "", "address to serve pprof profiles")
-	flagTest := flags.Bool("test", false, "enable image testing mode") // used by syz-ci
-	flagExecutor := flags.String("executor", "./syz-executor", "path to executor binary")
-	flagSignal := flags.Bool("cover", false, "collect feedback signals (coverage)")
-	flagSandbox := flags.String("sandbox", "none", "sandbox for fuzzing (none/setuid/namespace/android)")
-	flagDebug := flags.Bool("debug", false, "debug output from executor")
-	flagV := flags.Int("v", 0, "verbosity")
-	cmdLine := OldFuzzerCmd(os.Args[0], "/myexecutor", "myname", targets.Linux, targets.I386, "localhost:1234",
-		"namespace", 23, 3, true, true, false, 5)
-	args := strings.Split(cmdLine, " ")[1:]
-	if err := flags.Parse(args); err != nil {
-		t.Fatal(err)
-	}
-	if *flagName != "myname" {
-		t.Errorf("bad name: %q, want: %q", *flagName, "myname")
-	}
-	if *flagArch != targets.I386 {
-		t.Errorf("bad arch: %q, want: %q", *flagArch, targets.I386)
-	}
-	if *flagManager != "localhost:1234" {
-		t.Errorf("bad manager: %q, want: %q", *flagManager, "localhost:1234")
-	}
-	if *flagProcs != 3 {
-		t.Errorf("bad procs: %v, want: %v", *flagProcs, 3)
-	}
-	if *flagLeak {
-		t.Errorf("bad leak: %v, want: %v", *flagLeak, false)
-	}
-	if *flagOutput != "stdout" {
-		t.Errorf("bad output: %q, want: %q", *flagOutput, "stdout")
-	}
-	if *flagPprof != "" {
-		t.Errorf("bad pprof: %q, want: %q", *flagPprof, "")
-	}
-	if !*flagTest {
-		t.Errorf("bad test: %v, want: %v", *flagTest, true)
-	}
-	if *flagExecutor != "/myexecutor" {
-		t.Errorf("bad executor: %q, want: %q", *flagExecutor, "/myexecutor")
-	}
-	if *flagSandbox != "namespace" {
-		t.Errorf("bad sandbox: %q, want: %q", *flagSandbox, "namespace")
-	}
-	if !*flagSignal {
-		t.Errorf("bad signal: %v, want: %v", *flagSignal, true)
-	}
-	if *flagDebug {
-		t.Errorf("bad debug: %v, want: %v", *flagDebug, false)
-	}
-	if *flagV != 0 {
-		t.Errorf("bad verbosity: %v, want: %v", *flagV, 0)
-	}
-}
-
 func TestExecprogCmd(t *testing.T) {
 	// IMPORTANT: if this test fails, do not fix it by changing flags here!
 	// See comment in TestFuzzerCmd.