[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>

fuzz/sys_posix.go

@@ -88,5 +88,44 @@ func isInterruptError(err error) bool {
 		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
+	}
 }

fuzz/sys_unimplemented.go

@@ -34,3 +34,11 @@ func getWorkerComm() (comm workerComm, err error) {
 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")
+}

fuzz/sys_windows.go

@@ -140,3 +140,13 @@ func isInterruptError(err error) bool {
 	// 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")
+}

fuzz/worker.go

@@ -160,6 +160,14 @@ func (w *worker) coordinate(ctx context.Context) error {
 					// 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)