From 904f046e2ba812e04230c6e5252b3ca87c41e0e1 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] 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 c54bb57da2..c2cab7c813 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 436c18c126..c9f57a7ba4 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) @@ -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 index 0000000000..6036f45bc8 --- /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 71ba4ce416..8518276867 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.30.9