]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.9] runtime: restore the Go-allocated signal stack in unminit
authorAustin Clements <austin@google.com>
Fri, 1 Dec 2017 03:09:35 +0000 (22:09 -0500)
committerAndrew Bonventre <andybons@golang.org>
Mon, 22 Jan 2018 20:25:09 +0000 (20:25 +0000)
Currently, when we minit on a thread that already has an alternate
signal stack (e.g., because the M was an extram being used for a cgo
callback, or to handle a signal on a C thread, or because the
platform's libc always allocates a signal stack like on Android), we
simply drop the Go-allocated gsignal stack on the floor.

This is a problem for Ms on the extram list because those Ms may later
be reused for a different thread that may not have its own alternate
signal stack. On tip, this manifests as a crash in sigaltstack because
we clear the gsignal stack bounds in unminit and later try to use
those cleared bounds when we re-minit that M. On 1.9 and earlier, we
didn't clear the bounds, so this manifests as running more than one
signal handler on the same signal stack, which could lead to arbitrary
memory corruption.

This CL fixes this problem by saving the Go-allocated gsignal stack in
a new field in the m struct when overwriting it with a system-provided
signal stack, and then restoring the original gsignal stack in
unminit.

This CL is designed to be easy to back-port to 1.9. It won't quite
cherry-pick cleanly, but it should be sufficient to simply ignore the
change in mexit (which didn't exist in 1.9).

Now that we always have a place to stash the original signal stack in
the m struct, there are some simplifications we can make to the signal
stack handling. We'll do those in a later CL.

Fixes #22930.

Change-Id: I55c5a6dd9d97532f131146afdef0b216e1433054
Reviewed-on: https://go-review.googlesource.com/88315
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/runtime/crash_cgo_test.go
src/runtime/os3_plan9.go
src/runtime/os_nacl.go
src/runtime/runtime2.go
src/runtime/signal_unix.go
src/runtime/signal_windows.go
src/runtime/testdata/testprogcgo/sigstack.go [new file with mode: 0644]

index a5cbbad69bbfd66e0621e54e2a3e6eca86154eb4..57fe7965039b9fe5bc9d17bc522d4e2e844f8ec4 100644 (file)
@@ -411,3 +411,16 @@ func TestCgoNumGoroutine(t *testing.T) {
                t.Errorf("expected %q got %v", want, got)
        }
 }
+
+func TestSigStackSwapping(t *testing.T) {
+       switch runtime.GOOS {
+       case "plan9", "windows":
+               t.Skip("no sigaltstack on %s", runtime.GOOS)
+       }
+       t.Parallel()
+       got := runTestProg(t, "testprogcgo", "SigStack")
+       want := "OK\n"
+       if got != want {
+               t.Errorf("expected %q got %v", want, got)
+       }
+}
index 5d4b5a6698fac0c514bde81c070d63f183e391df..3b65a2c9ba18422d495514260291ba6cd659f08f 100644 (file)
@@ -153,3 +153,6 @@ func setThreadCPUProfiler(hz int32) {
        // TODO: Enable profiling interrupts.
        getg().m.profilehz = hz
 }
+
+// gsignalStack is unused on Plan 9.
+type gsignalStack struct{}
index 18e6ce62324bdf9e452128cf955d81fcf8714b34..12848327061a5d6c11158e3b93aaac4181be5728 100644 (file)
@@ -285,6 +285,9 @@ func sigenable(uint32)                                    {}
 func sigignore(uint32)                                    {}
 func closeonexec(int32)                                   {}
 
+// gsignalStack is unused on nacl.
+type gsignalStack struct{}
+
 var writelock uint32 // test-and-set spin lock for write
 
 /*
index 6871d9c68cd67e7c9a65c926be1804a15cc196ff..90364edb841fef9bb5a698bb8037b88fc9a4a51b 100644 (file)
@@ -386,10 +386,11 @@ type m struct {
        divmod  uint32 // div/mod denominator for arm - known to liblink
 
        // Fields not known to debuggers.
-       procid        uint64     // for debuggers, but offset not hard-coded
-       gsignal       *g         // signal-handling g
-       sigmask       sigset     // storage for saved signal mask
-       tls           [6]uintptr // thread-local storage (for x86 extern register)
+       procid        uint64       // for debuggers, but offset not hard-coded
+       gsignal       *g           // signal-handling g
+       goSigStack    gsignalStack // Go-allocated signal handling stack
+       sigmask       sigset       // storage for saved signal mask
+       tls           [6]uintptr   // thread-local storage (for x86 extern register)
        mstartfn      func()
        curg          *g       // current running goroutine
        caughtsig     guintptr // goroutine running during fatal signal
index 539b165ba1c6cd458c06841e631482af9b3e0a26..a495634e691bf74fe53f9c54fa76f8bef83b8a04 100644 (file)
@@ -702,7 +702,7 @@ func minitSignalStack() {
                signalstack(&_g_.m.gsignal.stack)
                _g_.m.newSigstack = true
        } else {
-               setGsignalStack(&st, nil)
+               setGsignalStack(&st, &_g_.m.goSigStack)
                _g_.m.newSigstack = false
        }
 }
@@ -732,6 +732,14 @@ func unminitSignals() {
        if getg().m.newSigstack {
                st := stackt{ss_flags: _SS_DISABLE}
                sigaltstack(&st, nil)
+       } else {
+               // We got the signal stack from someone else. Restore
+               // the Go-allocated stack in case this M gets reused
+               // for another thread (e.g., it's an extram). Also, on
+               // Android, libc allocates a signal stack for all
+               // threads, so it's important to restore the Go stack
+               // even on Go-created threads so we can free it.
+               restoreGsignalStack(&getg().m.goSigStack)
        }
 }
 
index 73bd5b5cfcedc5e306ef14fd213e3a35e2ed3a1b..8a63e58e54c9e3f4005e1de01b947e809d9be566 100644 (file)
@@ -223,3 +223,6 @@ func crash() {
        // It's okay to leave this empty for now: if crash returns
        // the ordinary exit-after-panic happens.
 }
+
+// gsignalStack is unused on Windows.
+type gsignalStack struct{}
diff --git a/src/runtime/testdata/testprogcgo/sigstack.go b/src/runtime/testdata/testprogcgo/sigstack.go
new file mode 100644 (file)
index 0000000..526ed42
--- /dev/null
@@ -0,0 +1,95 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !plan9,!windows
+
+// Test handling of Go-allocated signal stacks when calling from
+// C-created threads with and without signal stacks. (See issue
+// #22930.)
+
+package main
+
+/*
+#include <pthread.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+
+#ifndef MAP_STACK
+#define MAP_STACK 0
+#endif
+
+extern void SigStackCallback();
+
+static void* WithSigStack(void* arg __attribute__((unused))) {
+       // Set up an alternate system stack.
+       void* base = mmap(0, SIGSTKSZ, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0);
+       if (base == MAP_FAILED) {
+               perror("mmap failed");
+               abort();
+       }
+       stack_t st = {}, ost = {};
+       st.ss_sp = (char*)base;
+       st.ss_flags = 0;
+       st.ss_size = SIGSTKSZ;
+       if (sigaltstack(&st, &ost) < 0) {
+               perror("sigaltstack failed");
+               abort();
+       }
+
+       // Call Go.
+       SigStackCallback();
+
+       // Disable signal stack and protect it so we can detect reuse.
+       if (ost.ss_flags & SS_DISABLE) {
+               // Darwin libsystem has a bug where it checks ss_size
+               // even if SS_DISABLE is set. (The kernel gets it right.)
+               ost.ss_size = SIGSTKSZ;
+       }
+       if (sigaltstack(&ost, NULL) < 0) {
+               perror("sigaltstack restore failed");
+               abort();
+       }
+       mprotect(base, SIGSTKSZ, PROT_NONE);
+       return NULL;
+}
+
+static void* WithoutSigStack(void* arg __attribute__((unused))) {
+       SigStackCallback();
+       return NULL;
+}
+
+static void DoThread(int sigstack) {
+       pthread_t tid;
+       if (sigstack) {
+               pthread_create(&tid, NULL, WithSigStack, NULL);
+       } else {
+               pthread_create(&tid, NULL, WithoutSigStack, NULL);
+       }
+       pthread_join(tid, NULL);
+}
+*/
+import "C"
+
+func init() {
+       register("SigStack", SigStack)
+}
+
+func SigStack() {
+       C.DoThread(0)
+       C.DoThread(1)
+       C.DoThread(0)
+       C.DoThread(1)
+       println("OK")
+}
+
+var BadPtr *int
+
+//export SigStackCallback
+func SigStackCallback() {
+       // Cause the Go signal handler to run.
+       defer func() { recover() }()
+       *BadPtr = 42
+}