Skip to content

Commit 87f97c7

Browse files
committed
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 <[email protected]>
1 parent e95bc5f commit 87f97c7

File tree

2 files changed

+13
-0
lines changed

2 files changed

+13
-0
lines changed

src/runtime/proc1.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,13 @@ func scang(gp *g) {
414414
// the goroutine until we're done.
415415
if castogscanstatus(gp, s, s|_Gscan) {
416416
if !gp.gcscandone {
417+
// Coordinate with traceback
418+
// in sigprof.
419+
for !cas(&gp.stackLock, 0, 1) {
420+
osyield()
421+
}
417422
scanstack(gp)
423+
atomicstore(&gp.stackLock, 0)
418424
gp.gcscandone = true
419425
}
420426
restartg(gp)
@@ -2477,6 +2483,11 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
24772483
// Profiling runs concurrently with GC, so it must not allocate.
24782484
mp.mallocing++
24792485

2486+
// Coordinate with stack barrier insertion in scanstack.
2487+
for !cas(&gp.stackLock, 0, 1) {
2488+
osyield()
2489+
}
2490+
24802491
// Define that a "user g" is a user-created goroutine, and a "system g"
24812492
// is one that is m->g0 or m->gsignal.
24822493
//
@@ -2580,6 +2591,7 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
25802591
}
25812592
}
25822593
}
2594+
atomicstore(&gp.stackLock, 0)
25832595

25842596
if prof.hz != 0 {
25852597
// Simple cas-lock to coordinate with setcpuprofilerate.

src/runtime/runtime2.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ type g struct {
230230
stkbarPos uintptr // index of lowest stack barrier not hit
231231
param unsafe.Pointer // passed parameter on wakeup
232232
atomicstatus uint32
233+
stackLock uint32 // sigprof/scang lock; TODO: fold in to atomicstatus
233234
goid int64
234235
waitsince int64 // approx time when the g become blocked
235236
waitreason string // if status==Gwaiting

0 commit comments

Comments
 (0)