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.

1 files changed, 44 insertions, 76 deletions

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