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/12748
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index fccc2ac70fe4d8332b27d1f5308b3a685eeb04af..788f4fd3b4c295df42ce7c908807a0ba2ce8377c 100644 (file)
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -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.

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index 0132766dd0b754400665758963b415de9331f3b4..dc600ae5782375cfe4efb8b417f4a85aba4dfcf1 100644 (file)
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -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