Skip to content

Commit 904f046

Browse files
nyuichiianlancetaylor
authored andcommitted
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: #32912 (comment). This patch not only fixes the issue on AArch64 but also that on 32bit ARM. Fixes #32912 Change-Id: I657472e54b7aa3c617fabc5019ce63aa4105624a GitHub-Last-Rev: 28ce42c GitHub-Pull-Request: #34030 Reviewed-on: https://go-review.googlesource.com/c/go/+/192937 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 8ef6d6a commit 904f046

File tree

4 files changed

+86
-6
lines changed

4 files changed

+86
-6
lines changed

src/runtime/crash_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,15 @@ func buildTestProg(t *testing.T, binary string, flags ...string) (string, error)
143143
return exe, nil
144144
}
145145

146+
func TestVDSO(t *testing.T) {
147+
t.Parallel()
148+
output := runTestProg(t, "testprog", "SignalInVDSO")
149+
want := "success\n"
150+
if output != want {
151+
t.Fatalf("output:\n%s\n\nwanted:\n%s", output, want);
152+
}
153+
}
154+
146155
var (
147156
staleRuntimeOnce sync.Once // guards init of staleRuntimeErr
148157
staleRuntimeErr error

src/runtime/signal_unix.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,21 @@ func sigpipe() {
274274
dieFromSignal(_SIGPIPE)
275275
}
276276

277+
// sigFetchG fetches the value of G safely when running in a signal handler.
278+
// On some architectures, the g value may be clobbered when running in a VDSO.
279+
// See issue #32912.
280+
//
281+
//go:nosplit
282+
func sigFetchG(c *sigctxt) *g {
283+
switch GOARCH {
284+
case "arm", "arm64", "ppc64", "ppc64le":
285+
if inVDSOPage(c.sigpc()) {
286+
return nil
287+
}
288+
}
289+
return getg()
290+
}
291+
277292
// sigtrampgo is called from the signal handler function, sigtramp,
278293
// written in assembly code.
279294
// 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) {
289304
if sigfwdgo(sig, info, ctx) {
290305
return
291306
}
292-
g := getg()
307+
c := &sigctxt{info, ctx}
308+
g := sigFetchG(c)
293309
if g == nil {
294-
c := &sigctxt{info, ctx}
295310
if sig == _SIGPROF {
296311
sigprofNonGoPC(c.sigpc())
297312
return
@@ -347,7 +362,6 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
347362
signalDuringFork(sig)
348363
}
349364

350-
c := &sigctxt{info, ctx}
351365
c.fixsigcode(sig)
352366
sighandler(sig, info, ctx, g)
353367
setg(g)
@@ -657,9 +671,10 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
657671
return false
658672
}
659673
// Determine if the signal occurred inside Go code. We test that:
660-
// (1) we were in a goroutine (i.e., m.curg != nil), and
661-
// (2) we weren't in CGO.
662-
g := getg()
674+
// (1) we weren't in VDSO page,
675+
// (2) we were in a goroutine (i.e., m.curg != nil), and
676+
// (3) we weren't in CGO.
677+
g := sigFetchG(c)
663678
if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo {
664679
return false
665680
}

src/runtime/testdata/testprog/vdso.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Invoke signal hander in the VDSO context (see issue 32912).
6+
7+
package main
8+
9+
import (
10+
"fmt"
11+
"io/ioutil"
12+
"os"
13+
"runtime/pprof"
14+
"time"
15+
)
16+
17+
func init() {
18+
register("SignalInVDSO", signalInVDSO)
19+
}
20+
21+
func signalInVDSO() {
22+
f, err := ioutil.TempFile("", "timeprofnow")
23+
if err != nil {
24+
fmt.Fprintln(os.Stderr, err)
25+
os.Exit(2)
26+
}
27+
28+
if err := pprof.StartCPUProfile(f); err != nil {
29+
fmt.Fprintln(os.Stderr, err)
30+
os.Exit(2)
31+
}
32+
33+
t0 := time.Now()
34+
t1 := t0
35+
// We should get a profiling signal 100 times a second,
36+
// so running for 1 second should be sufficient.
37+
for t1.Sub(t0) < time.Second {
38+
t1 = time.Now()
39+
}
40+
41+
pprof.StopCPUProfile()
42+
43+
name := f.Name()
44+
if err := f.Close(); err != nil {
45+
fmt.Fprintln(os.Stderr, err)
46+
os.Exit(2)
47+
}
48+
49+
if err := os.Remove(name); err != nil {
50+
fmt.Fprintln(os.Stderr, err)
51+
os.Exit(2)
52+
}
53+
54+
fmt.Println("success");
55+
}

src/runtime/vdso_linux.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ func vdsoauxv(tag, val uintptr) {
281281
}
282282

283283
// vdsoMarker reports whether PC is on the VDSO page.
284+
//go:nosplit
284285
func inVDSOPage(pc uintptr) bool {
285286
for _, k := range vdsoSymbolKeys {
286287
if *k.ptr != 0 {

0 commit comments

Comments
 (0)