From: Michael Pratt <mpratt@google.com>
Date: Wed, 2 Jun 2021 21:44:43 +0000 (-0400)
Subject: runtime: avoid gp.lockedm race in exitsyscall0
X-Git-Tag: go1.17beta1~46
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=e0d029f75846f84f79e63f6100c57047f4a3fa98;p=gostls13.git

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 ded406cc28..59160c6525 100644
--- 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