From 25e10a043498087f9427f0698b341d051c310fc4 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Thu, 31 Jan 2019 10:57:46 +0100 Subject: executor: remove ability to detect kernel bugs This ability was never used but we maintain a bunch of code for it. syzkaller also recently learned to spoof this error code with some ptrace magic (probably intercepted control flow again and exploited executor binary). Drop all of it. --- executor/common.h | 4 +--- executor/executor.cc | 17 +---------------- pkg/csource/csource.go | 1 - pkg/csource/generated.go | 2 -- pkg/ipc/ipc.go | 14 ++++---------- pkg/ipc/ipc_test.go | 11 ++--------- pkg/report/linux.go | 5 ----- pkg/report/testdata/linux/report/128 | 3 --- pkg/runtest/run.go | 7 +------ sys/linux/init_test.go | 6 +++--- sys/targets/common.go | 4 ++-- syz-fuzzer/proc.go | 9 ++------- syz-fuzzer/testing.go | 5 +---- tools/syz-execprog/execprog.go | 8 ++------ tools/syz-stress/stress.go | 6 +++--- 15 files changed, 22 insertions(+), 80 deletions(-) delete mode 100644 pkg/report/testdata/linux/report/128 diff --git a/executor/common.h b/executor/common.h index dc1276cf9..cf4a204fc 100644 --- a/executor/common.h +++ b/executor/common.h @@ -7,7 +7,7 @@ // - includes are hoisted to the top and deduplicated // - comments and empty lines are stripped // - NORETURN/PRINTF/debug are removed -// - exitf/failf/fail are replaced with exit +// - exitf/fail are replaced with exit // - uintN types are replaced with uintN_t // - [[FOO]] placeholders are replaced by actual values @@ -614,8 +614,6 @@ static void loop(void) status = WEXITSTATUS(status); if (status == kFailStatus) fail("child failed"); - if (status == kErrorStatus) - error("child errored"); reply_execute(0); #endif #if SYZ_EXECUTOR || SYZ_USE_TMP_DIR diff --git a/executor/executor.cc b/executor/executor.cc index d1d1f2694..798cae71e 100644 --- a/executor/executor.cc +++ b/executor/executor.cc @@ -60,12 +60,9 @@ const int kMaxArgs = 9; const int kCoverSize = 256 << 10; const int kFailStatus = 67; const int kRetryStatus = 69; -const int kErrorStatus = 68; // Logical error (e.g. invalid input program), use as an assert() alternative. static NORETURN PRINTF(1, 2) void fail(const char* msg, ...); -// Kernel error (e.g. wrong syscall return value). -NORETURN PRINTF(1, 2) void error(const char* msg, ...); // Just exit (e.g. due to temporal ENOMEM error). static NORETURN PRINTF(1, 2) void exitf(const char* msg, ...); static NORETURN void doexit(int status); @@ -404,7 +401,7 @@ int main(int argc, char** argv) } #if SYZ_EXECUTOR_USES_FORK_SERVER // Other statuses happen when fuzzer processes manages to kill loop. - if (status != kFailStatus && status != kErrorStatus) + if (status != kFailStatus) status = kRetryStatus; // If an external sandbox process wraps executor, the out pipe will be closed // before the sandbox process exits this will make ipc package kill the sandbox. @@ -415,8 +412,6 @@ int main(int argc, char** argv) errno = 0; if (status == kFailStatus) fail("loop failed"); - if (status == kErrorStatus) - error("loop errored"); // Loop can be killed by a test process with e.g.: // ptrace(PTRACE_SEIZE, 1, 0, 0x100040) // This is unfortunate, but I don't have a better solution than ignoring it for now. @@ -1353,16 +1348,6 @@ void fail(const char* msg, ...) doexit((e == ENOMEM || e == EAGAIN) ? kRetryStatus : kFailStatus); } -void error(const char* msg, ...) -{ - va_list args; - va_start(args, msg); - vfprintf(stderr, msg, args); - va_end(args); - fprintf(stderr, "\n"); - doexit(kErrorStatus); -} - void exitf(const char* msg, ...) { int e = errno; diff --git a/pkg/csource/csource.go b/pkg/csource/csource.go index c9ebce62f..bcb40472a 100644 --- a/pkg/csource/csource.go +++ b/pkg/csource/csource.go @@ -422,7 +422,6 @@ func (ctx *context) postProcess(result []byte) []byte { result = regexp.MustCompile(`\t*debug_dump_data\((.*\n)*?.*\);\n`).ReplaceAll(result, nil) result = regexp.MustCompile(`\t*exitf\((.*\n)*?.*\);\n`).ReplaceAll(result, []byte("\texit(1);\n")) result = regexp.MustCompile(`\t*fail\((.*\n)*?.*\);\n`).ReplaceAll(result, []byte("\texit(1);\n")) - result = regexp.MustCompile(`\t*error\((.*\n)*?.*\);\n`).ReplaceAll(result, []byte("\texit(1);\n")) result = ctx.hoistIncludes(result) result = ctx.removeEmptyLines(result) diff --git a/pkg/csource/generated.go b/pkg/csource/generated.go index b8ba20bc2..5a402b66f 100644 --- a/pkg/csource/generated.go +++ b/pkg/csource/generated.go @@ -4666,8 +4666,6 @@ static void loop(void) status = WEXITSTATUS(status); if (status == kFailStatus) fail("child failed"); - if (status == kErrorStatus) - error("child errored"); reply_execute(0); #endif #if SYZ_EXECUTOR || SYZ_USE_TMP_DIR diff --git a/pkg/ipc/ipc.go b/pkg/ipc/ipc.go index ca0f5ac32..877fe5c06 100644 --- a/pkg/ipc/ipc.go +++ b/pkg/ipc/ipc.go @@ -121,7 +121,6 @@ const ( outputSize = 16 << 20 statusFail = 67 - statusError = 68 statusRetry = 69 // Comparison types masks taken from KCOV headers. @@ -248,10 +247,9 @@ var rateLimit = time.NewTicker(1 * time.Second) // Exec starts executor binary to execute program p and returns information about the execution: // output: process output // info: per-call info -// failed: true if executor has detected a kernel bug // hanged: program hanged and was killed -// err0: failed to start process, or executor has detected a logical error -func (env *Env) Exec(opts *ExecOpts, p *prog.Prog) (output []byte, info *ProgInfo, failed, hanged bool, err0 error) { +// err0: failed to start the process or bug in executor itself +func (env *Env) Exec(opts *ExecOpts, p *prog.Prog) (output []byte, info *ProgInfo, hanged bool, err0 error) { // Copy-in serialized program. progSize, err := p.SerializeForExec(env.in) if err != nil { @@ -282,7 +280,7 @@ func (env *Env) Exec(opts *ExecOpts, p *prog.Prog) (output []byte, info *ProgInf } } var restart bool - output, failed, hanged, restart, err0 = env.cmd.exec(opts, progData) + output, hanged, restart, err0 = env.cmd.exec(opts, progData) if err0 != nil { env.cmd.close() env.cmd = nil @@ -717,8 +715,7 @@ func (c *command) wait() error { return err } -func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, failed, hanged, - restart bool, err0 error) { +func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, hanged, restart bool, err0 error) { req := &executeReq{ magic: inMagic, envFlags: uint64(c.config.Flags), @@ -812,9 +809,6 @@ func (c *command) exec(opts *ExecOpts, progData []byte) (output []byte, failed, switch exitStatus { case statusFail: err0 = ExecutorFailure(fmt.Sprintf("executor %v: failed: %s", c.pid, output)) - case statusError: - err0 = fmt.Errorf("executor %v: detected kernel bug", c.pid) - failed = true case statusRetry: // This is a temporal error (ENOMEM) or an unfortunate // program that messes with testing setup (e.g. kills executor diff --git a/pkg/ipc/ipc_test.go b/pkg/ipc/ipc_test.go index d168a2535..8d0ca7ff5 100644 --- a/pkg/ipc/ipc_test.go +++ b/pkg/ipc/ipc_test.go @@ -96,16 +96,13 @@ func TestExecute(t *testing.T) { opts := &ExecOpts{ Flags: flag, } - output, info, failed, hanged, err := env.Exec(opts, p) + output, info, hanged, err := env.Exec(opts, p) if err != nil { t.Fatalf("failed to run executor: %v", err) } if hanged { t.Fatalf("program hanged:\n%s", output) } - if failed { - t.Fatalf("program failed:\n%s", output) - } if len(info.Calls) == 0 { t.Fatalf("no calls executed:\n%s", output) } @@ -142,7 +139,7 @@ func TestParallel(t *testing.T) { }() p := target.GenerateSimpleProg() opts := &ExecOpts{} - output, info, failed, hanged, err := env.Exec(opts, p) + output, info, hanged, err := env.Exec(opts, p) if err != nil { err = fmt.Errorf("failed to run executor: %v", err) return @@ -151,10 +148,6 @@ func TestParallel(t *testing.T) { err = fmt.Errorf("program hanged:\n%s", output) return } - if failed { - err = fmt.Errorf("program failed:\n%s", output) - return - } if len(info.Calls) == 0 { err = fmt.Errorf("no calls executed:\n%s", output) return diff --git a/pkg/report/linux.go b/pkg/report/linux.go index c35f689bf..fcf75d488 100644 --- a/pkg/report/linux.go +++ b/pkg/report/linux.go @@ -943,11 +943,6 @@ var linuxOopses = []*oops{ fmt: "BUG: workqueue leaked lock or atomic in %[1]v", noStackTrace: true, }, - { - title: compile("BUG: executor-detected bug"), - fmt: "BUG: executor-detected bug", - noStackTrace: true, - }, { title: compile("BUG: memory leak"), fmt: MemoryLeakPrefix + "%[1]v", diff --git a/pkg/report/testdata/linux/report/128 b/pkg/report/testdata/linux/report/128 deleted file mode 100644 index bc9f2c9b6..000000000 --- a/pkg/report/testdata/linux/report/128 +++ /dev/null @@ -1,3 +0,0 @@ -TITLE: BUG: executor-detected bug - -BUG: executor-detected bug \ No newline at end of file diff --git a/pkg/runtest/run.go b/pkg/runtest/run.go index 043019d81..de5061539 100644 --- a/pkg/runtest/run.go +++ b/pkg/runtest/run.go @@ -511,17 +511,12 @@ func RunTest(req *RunRequest, executor string) { } defer env.Close() for run := 0; run < req.Repeat; run++ { - output, info, failed, hanged, err := env.Exec(req.Opts, req.P) + output, info, hanged, err := env.Exec(req.Opts, req.P) req.Output = append(req.Output, output...) if err != nil { req.Err = fmt.Errorf("run %v: failed to run: %v", run, err) return } - if failed { - req.Err = fmt.Errorf("run %v: failed", run) - return - } - if hanged { req.Err = fmt.Errorf("run %v: hanged", run) return diff --git a/sys/linux/init_test.go b/sys/linux/init_test.go index 104d3c068..fb81c1140 100644 --- a/sys/linux/init_test.go +++ b/sys/linux/init_test.go @@ -121,10 +121,10 @@ mknod(0x0, 0x6000, 0x700) exit(0x3) exit(0x43) exit(0xc3) -exit(0xc4) +exit(0xc3) exit_group(0x5a) -exit_group(0x44) -exit_group(0x444) +exit_group(0x43) +exit_group(0x443) `, ` exit(0x3) diff --git a/sys/targets/common.go b/sys/targets/common.go index 325e6ca88..0096bcf75 100644 --- a/sys/targets/common.go +++ b/sys/targets/common.go @@ -104,8 +104,8 @@ func (arch *UnixSanitizer) SanitizeCall(c *prog.Call) { } case "exit", "exit_group": code := c.Args[0].(*prog.ConstArg) - // These codes are reserved by executor. - if code.Val%128 == 67 || code.Val%128 == 68 { + // This code is reserved by executor. + if code.Val%128 == 67 { code.Val = 1 } } diff --git a/syz-fuzzer/proc.go b/syz-fuzzer/proc.go index 77af08de7..3b884b2ac 100644 --- a/syz-fuzzer/proc.go +++ b/syz-fuzzer/proc.go @@ -289,12 +289,7 @@ func (proc *Proc) executeRaw(opts *ipc.ExecOpts, p *prog.Prog, stat Stat) *ipc.P proc.logProgram(opts, p) for try := 0; ; try++ { atomic.AddUint64(&proc.fuzzer.stats[stat], 1) - output, info, failed, hanged, err := proc.env.Exec(opts, p) - if failed { - // BUG in output should be recognized by manager. - // Exit immediately so that the input is not added to corpus. - log.Fatalf("BUG: executor-detected bug\nproc %v\n%s", proc.pid, output) - } + output, info, hanged, err := proc.env.Exec(opts, p) if err != nil { if try > 10 { log.Fatalf("executor %v failed %v times:\n%v", proc.pid, try, err) @@ -304,7 +299,7 @@ func (proc *Proc) executeRaw(opts *ipc.ExecOpts, p *prog.Prog, stat Stat) *ipc.P time.Sleep(time.Second) continue } - log.Logf(2, "result failed=%v hanged=%v: %s\n", failed, hanged, output) + log.Logf(2, "result hanged=%v: %s", hanged, output) return info } } diff --git a/syz-fuzzer/testing.go b/syz-fuzzer/testing.go index 938800f64..aa8523154 100644 --- a/syz-fuzzer/testing.go +++ b/syz-fuzzer/testing.go @@ -228,16 +228,13 @@ func checkSimpleProgram(args *checkArgs) error { } defer env.Close() p := args.target.GenerateSimpleProg() - output, info, failed, hanged, err := env.Exec(args.ipcExecOpts, p) + output, info, hanged, err := env.Exec(args.ipcExecOpts, p) if err != nil { return fmt.Errorf("program execution failed: %v\n%s", err, output) } if hanged { return fmt.Errorf("program hanged:\n%s", output) } - if failed { - return fmt.Errorf("program failed:\n%s", output) - } if len(info.Calls) == 0 { return fmt.Errorf("no calls executed:\n%s", output) } diff --git a/tools/syz-execprog/execprog.go b/tools/syz-execprog/execprog.go index 82fa00906..f65ad9d9b 100644 --- a/tools/syz-execprog/execprog.go +++ b/tools/syz-execprog/execprog.go @@ -140,13 +140,9 @@ func (ctx *Context) execute(pid int, env *ipc.Env, entry *prog.LogEntry) { if *flagOutput { ctx.logProgram(pid, entry.P, callOpts) } - output, info, failed, hanged, err := env.Exec(callOpts, entry.P) - if failed { - log.Logf(0, "BUG: executor-detected bug:\n%s", output) - } + output, info, hanged, err := env.Exec(callOpts, entry.P) if ctx.config.Flags&ipc.FlagDebug != 0 || err != nil { - log.Logf(0, "result: failed=%v hanged=%v err=%v\n\n%s", - failed, hanged, err, output) + log.Logf(0, "result: hanged=%v err=%v\n\n%s", hanged, err, output) } if info != nil { ctx.printCallResults(info) diff --git a/tools/syz-stress/stress.go b/tools/syz-stress/stress.go index e5f4ff845..d1beee34d 100644 --- a/tools/syz-stress/stress.go +++ b/tools/syz-stress/stress.go @@ -114,14 +114,14 @@ func execute(pid int, env *ipc.Env, execOpts *ipc.ExecOpts, p *prog.Prog) { fmt.Printf("executing program %v\n%s\n", pid, p.Serialize()) outMu.Unlock() } - output, _, failed, hanged, err := env.Exec(execOpts, p) + output, _, hanged, err := env.Exec(execOpts, p) if err != nil { fmt.Printf("failed to execute executor: %v\n", err) } - if failed || hanged || err != nil || *flagOutput { + if hanged || err != nil || *flagOutput { fmt.Printf("PROGRAM:\n%s\n", p.Serialize()) } - if failed || hanged || err != nil || *flagOutput { + if hanged || err != nil || *flagOutput { os.Stdout.Write(output) } } -- cgit mrf-deployment