]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.fuzz] internal/fuzz: improve handling of worker termination by signal
authorJay Conrod <jayconrod@google.com>
Wed, 7 Jul 2021 23:27:22 +0000 (16:27 -0700)
committerJay Conrod <jayconrod@google.com>
Thu, 8 Jul 2021 16:39:05 +0000 (16:39 +0000)
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>

src/cmd/go/testdata/script/test_fuzz_non_crash_signal.txt [new file with mode: 0644]
src/internal/fuzz/sys_posix.go
src/internal/fuzz/sys_unimplemented.go
src/internal/fuzz/sys_windows.go
src/internal/fuzz/worker.go

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 (file)
index 0000000..a67bf63
--- /dev/null
@@ -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{}
+       })
+}
index 8ea84d2025e278264658a427244146d840c5ba86..2473274ecf26127c66e691b8b1dfecb491da0c0e 100644 (file)
@@ -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
+       }
 }
index 5f80379c223893d5f4be3b95b353e82e388cc977..827e36cf3297b4d3f53e35e9410ff2efa2aaa8ac 100644 (file)
@@ -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")
+}
index 286501bc10f556c1d3e364b1f551aee83e13e67b..fabf954ba735cc96cdc23db61db683eb0ffb8766 100644 (file)
@@ -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")
+}
index e1fc999104bb79031ac17dbf8bd978bd865bee10..c3f4d743023f3f5f3a24f28dee55b14fb76652a5 100644 (file)
@@ -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)