[dev.fuzz] internal/fuzz: improve handling of worker termination by signal
With this change, we'll no longer silently ignore terminations by
SIGKILL. We use SIGKILL to terminate unresponsive workers, but it can
also be delivered by the OOM killer.
When a worker is terminated by a signal not apparently due to a crash
or interruption (like SIGKILL or SIGHUP, as opposed to SIGSEGV), we'll
log a message, but we won't record a crash, since any given input is
not likely to reproduce this termination.
Fixes golang/go#46576
Change-Id: I6ef18a7cf5a457c7b9bc44cf5416378271216bfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/333190
Trust: Jay Conrod <jayconrod@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
diff --git a/src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt b/src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt
new file mode 100644
index 0000000..a67bf63
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt
@@ -0,0 +1,55 @@
+# NOTE: this test is skipped on Windows, since there's no concept of signals.
+# When a process terminates another process, it provides an exit code.
+# TODO(jayconrod): support shared memory on more platforms.
+[!darwin] [!freebsd] [!linux] skip
+[short] skip
+
+# FuzzNonCrash sends itself a signal that does not appear to be a crash.
+# We should not save a crasher.
+! go test -fuzz=FuzzNonCrash
+! exists testdata
+! stdout unreachable
+! stderr unreachable
+stdout 'fuzzing process terminated by unexpected signal; no crash will be recorded: signal: killed'
+
+# FuzzCrash sends itself a signal that looks like a crash.
+# We should save a crasher.
+! go test -fuzz=FuzzCrash
+exists testdata/corpus/FuzzCrash
+stdout 'fuzzing process terminated unexpectedly'
+
+-- go.mod --
+module test
+
+go 1.17
+-- fuzz_posix_test.go --
+// +build darwin freebsd linux
+
+package fuzz
+
+import (
+ "syscall"
+ "testing"
+)
+
+func FuzzNonCrash(f *testing.F) {
+ f.Fuzz(func(*testing.T, bool) {
+ pid := syscall.Getpid()
+ if err := syscall.Kill(pid, syscall.SIGKILL); err != nil {
+ panic(err)
+ }
+ // signal may not be received immediately. Wait for it.
+ select{}
+ })
+}
+
+func FuzzCrash(f *testing.F) {
+ f.Fuzz(func(*testing.T, bool) {
+ pid := syscall.Getpid()
+ if err := syscall.Kill(pid, syscall.SIGILL); err != nil {
+ panic(err)
+ }
+ // signal may not be received immediately. Wait for it.
+ select{}
+ })
+}
diff --git a/src/internal/fuzz/sys_posix.go b/src/internal/fuzz/sys_posix.go
index 8ea84d2..2473274 100644
--- a/src/internal/fuzz/sys_posix.go
+++ b/src/internal/fuzz/sys_posix.go
@@ -88,5 +88,44 @@
return false
}
status := exitErr.Sys().(syscall.WaitStatus)
- return status.Signal() == syscall.SIGINT || status.Signal() == syscall.SIGKILL
+ return status.Signal() == syscall.SIGINT
+}
+
+// terminationSignal checks if err is an exec.ExitError with a signal status.
+// If it is, terminationSignal returns the signal and true.
+// If not, -1 and false.
+func terminationSignal(err error) (os.Signal, bool) {
+ exitErr, ok := err.(*exec.ExitError)
+ if !ok || exitErr.ExitCode() >= 0 {
+ return syscall.Signal(-1), false
+ }
+ status := exitErr.Sys().(syscall.WaitStatus)
+ return status.Signal(), status.Signaled()
+}
+
+// isCrashSignal returns whether a signal was likely to have been caused by an
+// error in the program that received it, triggered by a fuzz input. For
+// example, SIGSEGV would be received after a nil pointer dereference.
+// Other signals like SIGKILL or SIGHUP are more likely to have been sent by
+// another process, and we shouldn't record a crasher if the worker process
+// receives one of these.
+//
+// Note that Go installs its own signal handlers on startup, so some of these
+// signals may only be received if signal handlers are changed. For example,
+// SIGSEGV is normally transformed into a panic that causes the process to exit
+// with status 2 if not recovered, which we handle as a crash.
+func isCrashSignal(signal os.Signal) bool {
+ switch signal {
+ case
+ syscall.SIGILL, // illegal instruction
+ syscall.SIGTRAP, // breakpoint
+ syscall.SIGABRT, // abort() called
+ syscall.SIGBUS, // invalid memory access (e.g., misaligned address)
+ syscall.SIGFPE, // math error, e.g., integer divide by zero
+ syscall.SIGSEGV, // invalid memory access (e.g., write to read-only)
+ syscall.SIGPIPE: // sent data to closed pipe or socket
+ return true
+ default:
+ return false
+ }
}
diff --git a/src/internal/fuzz/sys_unimplemented.go b/src/internal/fuzz/sys_unimplemented.go
index 5f80379..827e36c 100644
--- a/src/internal/fuzz/sys_unimplemented.go
+++ b/src/internal/fuzz/sys_unimplemented.go
@@ -34,3 +34,11 @@
func isInterruptError(err error) bool {
panic("not implemented")
}
+
+func terminationSignal(err error) (os.Signal, bool) {
+ panic("not implemented")
+}
+
+func isCrashSignal(signal os.Signal) bool {
+ panic("not implemented")
+}
diff --git a/src/internal/fuzz/sys_windows.go b/src/internal/fuzz/sys_windows.go
index 286501b..fabf954 100644
--- a/src/internal/fuzz/sys_windows.go
+++ b/src/internal/fuzz/sys_windows.go
@@ -140,3 +140,13 @@
// returned by Wait. It looks like an ExitError with status 1.
return false
}
+
+// terminationSignal returns -1 and false because Windows doesn't have signals.
+func terminationSignal(err error) (os.Signal, bool) {
+ return syscall.Signal(-1), false
+}
+
+// isCrashSignal is not implemented because Windows doesn't have signals.
+func isCrashSignal(signal os.Signal) bool {
+ panic("not implemented: no signals on windows")
+}
diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index e1fc999..c3f4d74 100644
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -160,6 +160,14 @@
// Since we expect I/O errors around interrupts, ignore this error.
return nil
}
+ if sig, ok := terminationSignal(w.waitErr); ok && !isCrashSignal(sig) {
+ // Worker terminated by a signal that probably wasn't caused by a
+ // specific input to the fuzz function. For example, on Linux,
+ // the kernel (OOM killer) may send SIGKILL to a process using a lot
+ // of memory. Or the shell might send SIGHUP when the terminal
+ // is closed. Don't record a crasher.
+ return fmt.Errorf("fuzzing process terminated by unexpected signal; no crash will be recorded: %v", w.waitErr)
+ }
// Unexpected termination. Set error message and fall through.
// We'll restart the worker on the next iteration.
resp.Err = fmt.Sprintf("fuzzing process terminated unexpectedly: %v", w.waitErr)
