runtime: avoid gp.lockedm race in exitsyscall0

Following https://golang.org/cl/291329, exitsyscall0 accesses gp.lockedm
after releasing gp to the global runq. This creates a race window where
another M may schedule the (unlocked) G, which subsequently calls
LockOSThread, setting gp.lockedm and thus causing exitsyscall0 to think
it should call stoplockedm.

Avoid this race by checking if gp is locked before releasing it to the
global runq.

Fixes #46524

Change-Id: I3acdaf09e7a2178725adbe61e985130e9ebd0680
Reviewed-on: https://go-review.googlesource.com/c/go/+/324350
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index ded406cc28dcc8c91098e248e7e45f18baac4fec..59160c6525fac2713e2ec8efd3a6089a4bea403b 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -4083,8 +4083,16 @@ func exitsyscall0(gp *g) {
        if schedEnabled(gp) {
                _p_ = pidleget()
        }
+       var locked bool
        if _p_ == nil {
                globrunqput(gp)
+
+               // Below, we stoplockedm if gp is locked. globrunqput releases
+               // ownership of gp, so we must check if gp is locked prior to
+               // committing the release by unlocking sched.lock, otherwise we
+               // could race with another M transitioning gp from unlocked to
+               // locked.
+               locked = gp.lockedm != 0
        } else if atomic.Load(&sched.sysmonwait) != 0 {
                atomic.Store(&sched.sysmonwait, 0)
                notewakeup(&sched.sysmonnote)
@@ -4094,7 +4102,7 @@ func exitsyscall0(gp *g) {
                acquirep(_p_)
                execute(gp, false) // Never returns.
        }
-       if gp.lockedm != 0 {
+       if locked {
                // Wait until another thread schedules gp and so m again.
                //
                // N.B. lockedm must be this M, as this g was running on this M