Commit 87f97c73 authored by Austin Clements's avatar Austin Clements

runtime: avoid race between SIGPROF traceback and stack barriers

The following sequence of events can lead to the runtime attempting an
out-of-bounds access on a stack barrier slice:

1. A SIGPROF comes in on a thread while the G on that thread is in
   _Gsyscall. The sigprof handler calls gentraceback, which saves a
   local copy of the G's stkbar slice. Currently the G has no stack
   barriers, so this slice is empty.

2. On another thread, the GC concurrently scans the stack of the
   goroutine being profiled (it considers it stopped because it's in
   _Gsyscall) and installs stack barriers.

3. Back on the sigprof thread, gentraceback comes across a stack
   barrier in the stack and attempts to look it up in its (zero
   length) copy of G's old stkbar slice, which causes an out-of-bounds
   access.

This commit fixes this by adding a simple cas spin to synchronize the
SIGPROF handler with stack barrier insertion.

In general I would prefer that this synchronization be done through
the G status, since that's how stack scans are otherwise synchronized,
but adding a new lock is a much smaller change and G statuses are full
of subtlety.

Fixes #11863.

Change-Id: Ie89614a6238bb9c6a5b1190499b0b48ec759eaf7
Reviewed-on: https://go-review.googlesource.com/12748Reviewed-by: default avatarRuss Cox <rsc@golang.org>
parent e95bc5fe
......@@ -414,7 +414,13 @@ func scang(gp *g) {
// the goroutine until we're done.
if castogscanstatus(gp, s, s|_Gscan) {
if !gp.gcscandone {
// Coordinate with traceback
// in sigprof.
for !cas(&gp.stackLock, 0, 1) {
osyield()
}
scanstack(gp)
atomicstore(&gp.stackLock, 0)
gp.gcscandone = true
}
restartg(gp)
......@@ -2477,6 +2483,11 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
// Profiling runs concurrently with GC, so it must not allocate.
mp.mallocing++
// Coordinate with stack barrier insertion in scanstack.
for !cas(&gp.stackLock, 0, 1) {
osyield()
}
// Define that a "user g" is a user-created goroutine, and a "system g"
// is one that is m->g0 or m->gsignal.
//
......@@ -2580,6 +2591,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
}
}
}
atomicstore(&gp.stackLock, 0)
if prof.hz != 0 {
// Simple cas-lock to coordinate with setcpuprofilerate.
......
......@@ -230,6 +230,7 @@ type g struct {
stkbarPos uintptr // index of lowest stack barrier not hit
param unsafe.Pointer // passed parameter on wakeup
atomicstatus uint32
stackLock uint32 // sigprof/scang lock; TODO: fold in to atomicstatus
goid int64
waitsince int64 // approx time when the g become blocked
waitreason string // if status==Gwaiting
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment