runtime: fix crash during VDSO calls on arm

As discussed in #32912, a crash occurs when go runtime calls a VDSO function (say
__vdso_clock_gettime) and a signal arrives to that thread.
Since VDSO functions temporarily destroy the G register (R10),
Go functions asynchronously executed in that thread (i.e. Go's signal
handler) can try to load data from the destroyed G, which causes
segmentation fault.

To fix the issue a guard is inserted in front of sigtrampgo, so that the control escapes from
signal handlers without touching G in case the signal occurred in the VDSO context.
The test case included in the patch is take from discussion in a relevant thread on github:
https://github.com/golang/go/issues/32912#issuecomment-517874531.
This patch not only fixes the issue on AArch64 but also that on 32bit ARM.

Fixes #32912

Change-Id: I657472e54b7aa3c617fabc5019ce63aa4105624a
GitHub-Last-Rev: 28ce42c4a02a060f08c1b0dd1c9a392123fd2ee9
GitHub-Pull-Request: golang/go#34030
Reviewed-on: https://go-review.googlesource.com/c/go/+/192937
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index c54bb57da2ee0b28119ae2f37d67552c60e531e3..c2cab7c813bf558696b7dadff3a61fd037c165bd 100644 (file)
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -143,6 +143,15 @@ func buildTestProg(t *testing.T, binary string, flags ...string) (string, error)
        return exe, nil
 }
 
+func TestVDSO(t *testing.T) {
+       t.Parallel()
+       output := runTestProg(t, "testprog", "SignalInVDSO")
+       want := "success\n"
+       if output != want {
+               t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want);
+       }
+}
+
 var (
        staleRuntimeOnce sync.Once // guards init of staleRuntimeErr
        staleRuntimeErr  error

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index 436c18c1261f5443f829b22a17bbb83cce9386c5..c9f57a7ba4531ea8daf48566f41831c3fde79b20 100644 (file)
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -274,6 +274,21 @@ func sigpipe() {
        dieFromSignal(_SIGPIPE)
 }
 
+// sigFetchG fetches the value of G safely when running in a signal handler.
+// On some architectures, the g value may be clobbered when running in a VDSO.
+// See issue #32912.
+//
+//go:nosplit
+func sigFetchG(c *sigctxt) *g {
+    switch GOARCH {
+    case "arm", "arm64", "ppc64", "ppc64le":
+        if inVDSOPage(c.sigpc()) {
+            return nil
+        }
+    }
+    return getg()
+}
+
 // sigtrampgo is called from the signal handler function, sigtramp,
 // written in assembly code.
 // This is called by the signal handler, and the world may be stopped.
@@ -289,9 +304,9 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
        if sigfwdgo(sig, info, ctx) {
                return
        }
-       g := getg()
+       c := &sigctxt{info, ctx}
+       g := sigFetchG(c)
        if g == nil {
-               c := &sigctxt{info, ctx}
                if sig == _SIGPROF {
                        sigprofNonGoPC(c.sigpc())
                        return
@@ -347,7 +362,6 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
                signalDuringFork(sig)
        }
 
-       c := &sigctxt{info, ctx}
        c.fixsigcode(sig)
        sighandler(sig, info, ctx, g)
        setg(g)
@@ -657,9 +671,10 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
                return false
        }
        // Determine if the signal occurred inside Go code. We test that:
-       //   (1) we were in a goroutine (i.e., m.curg != nil), and
-       //   (2) we weren't in CGO.
-       g := getg()
+       //   (1) we weren't in VDSO page,
+       //   (2) we were in a goroutine (i.e., m.curg != nil), and
+       //   (3) we weren't in CGO.
+       g := sigFetchG(c)
        if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo {
                return false
        }

diff --git a/src/runtime/testdata/testprog/vdso.go b/src/runtime/testdata/testprog/vdso.go
new file mode 100644 (file)
index 0000000..6036f45
--- /dev/null
+++ b/src/runtime/testdata/testprog/vdso.go
@@ -0,0 +1,55 @@
+// Copyright 2019 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.
+
+// Invoke signal hander in the VDSO context (see issue 32912).
+
+package main
+
+import (
+       "fmt"
+       "io/ioutil"
+       "os"
+       "runtime/pprof"
+       "time"
+)
+
+func init() {
+       register("SignalInVDSO", signalInVDSO)
+}
+
+func signalInVDSO() {
+        f, err := ioutil.TempFile("", "timeprofnow")
+        if err != nil {
+               fmt.Fprintln(os.Stderr, err)
+               os.Exit(2)
+        }
+
+        if err := pprof.StartCPUProfile(f); err != nil {
+               fmt.Fprintln(os.Stderr, err)
+               os.Exit(2)
+        }
+
+        t0 := time.Now()
+        t1 := t0
+        // We should get a profiling signal 100 times a second,
+        // so running for 1 second should be sufficient.
+        for t1.Sub(t0) < time.Second {
+                t1 = time.Now()
+        }
+
+        pprof.StopCPUProfile()
+
+        name := f.Name()
+        if err := f.Close(); err != nil {
+               fmt.Fprintln(os.Stderr, err)
+               os.Exit(2)
+        }
+
+        if err := os.Remove(name); err != nil {
+               fmt.Fprintln(os.Stderr, err)
+               os.Exit(2)
+        }
+
+       fmt.Println("success");
+}

diff --git a/src/runtime/vdso_linux.go b/src/runtime/vdso_linux.go
index 71ba4ce4161b665c6af4574fc7a4fdc1d5588936..8518276867e3949f69926ae3fa6e14b644552965 100644 (file)
--- a/src/runtime/vdso_linux.go
+++ b/src/runtime/vdso_linux.go
@@ -281,6 +281,7 @@ func vdsoauxv(tag, val uintptr) {
 }
 
 // vdsoMarker reports whether PC is on the VDSO page.
+//go:nosplit
 func inVDSOPage(pc uintptr) bool {
        for _, k := range vdsoSymbolKeys {
                if *k.ptr != 0 {