192 lines
5.8 KiB
Diff
192 lines
5.8 KiB
Diff
|
|
From 8a755c0f0389dca42ec8caef0efa9b6ebe9d1e3c Mon Sep 17 00:00:00 2001
|
||
|
|
From: Yuichi Nishiwaki <yuichi.nishiwaki@gmail.com>
|
||
|
|
Date: Wed, 11 Sep 2019 02:26:02 +0000
|
||
|
|
Subject: [PATCH 2/6] 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>
|
||
|
|
---
|
||
|
|
src/runtime/crash_test.go | 9 +++++
|
||
|
|
src/runtime/signal_unix.go | 27 ++++++++++---
|
||
|
|
src/runtime/testdata/testprog/vdso.go | 55 +++++++++++++++++++++++++++
|
||
|
|
src/runtime/vdso_linux.go | 1 +
|
||
|
|
4 files changed, 86 insertions(+), 6 deletions(-)
|
||
|
|
create mode 100644 src/runtime/testdata/testprog/vdso.go
|
||
|
|
|
||
|
|
diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
|
||
|
|
index c54bb57..c2cab7c 100644
|
||
|
|
--- 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 ad51dc1..63fb07f 100644
|
||
|
|
--- 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)
|
||
|
|
@@ -650,9 +664,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
|
||
|
|
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 71ba4ce..8518276 100644
|
||
|
|
--- 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 {
|
||
|
|
--
|
||
|
|
2.17.1
|
||
|
|
|