[release-branch.go1.21] runtime: clear g0 stack bounds in dropm

After CL 527715, needm uses callbackUpdateSystemStack to set the stack
bounds for g0 on an M from the extra M list. Since
callbackUpdateSystemStack is also used for recursive cgocallback, it
does nothing if the stack is already in bounds.

Currently, the stack bounds in an extra M may contain stale bounds from
a previous thread that used this M and then returned it to the extra
list in dropm.

Typically a new thread will not have an overlapping stack with an old
thread, but because the old thread has exited there is a small chance
that the C memory allocator will allocate the new thread's stack
partially or fully overlapping with the old thread's stack.

If this occurs, then callbackUpdateSystemStack will not update the stack
bounds. If in addition, the overlap is partial such that SP on
cgocallback is close to the recorded stack lower bound, then Go may
quickly "overflow" the stack and crash with "morestack on g0".

Fix this by clearing the stack bounds in dropm, which ensures that
callbackUpdateSystemStack will unconditionally update the bounds in
needm.

For #62440.
Fixes #63209.

Change-Id: Ic9e2052c2090dd679ed716d1a23a86d66cbcada7
Reviewed-on: https://go-review.googlesource.com/c/go/+/537695
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
(cherry picked from commit 1af424c196584cd0b05e559c2740f046d1f32042)
Reviewed-on: https://go-review.googlesource.com/c/go/+/549495
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>

2 files changed, 50 insertions, 1 deletions

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index eaea523ab5..0425aede23 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -2186,6 +2186,14 @@ func dropm() {
 
 	setg(nil)
 
+	// Clear g0 stack bounds to ensure that needm always refreshes the
+	// bounds when reusing this M.
+	g0 := mp.g0
+	g0.stack.hi = 0
+	g0.stack.lo = 0
+	g0.stackguard0 = 0
+	g0.stackguard1 = 0
+
 	putExtraM(mp)
 
 	msigrestore(sigmask)
diff --git a/src/runtime/testdata/testprogcgo/stackswitch.c b/src/runtime/testdata/testprogcgo/stackswitch.c
index 2f79cc28ed..3473d5bd57 100644
--- a/src/runtime/testdata/testprogcgo/stackswitch.c
+++ b/src/runtime/testdata/testprogcgo/stackswitch.c
@@ -43,6 +43,8 @@ static ucontext_t uctx_save, uctx_switch;
 
 extern void stackSwitchCallback(void);
 
+char *stack2;
+
 static void *stackSwitchThread(void *arg) {
 	// Simple test: callback works from the normal system stack.
 	stackSwitchCallback();
@@ -57,7 +59,9 @@ static void *stackSwitchThread(void *arg) {
 
 	// Allocate the second stack before freeing the first to ensure we don't get
 	// the same address from malloc.
-	char *stack2 = malloc(STACK_SIZE);
+	//
+	// Will be freed in stackSwitchThread2.
+	stack2 = malloc(STACK_SIZE);
 	if (stack1 == NULL) {
 		perror("malloc");
 		exit(1);
@@ -92,6 +96,40 @@ static void *stackSwitchThread(void *arg) {
 	}
 
 	free(stack1);
+
+	return NULL;
+}
+
+static void *stackSwitchThread2(void *arg) {
+	// New thread. Use stack bounds that partially overlap the previous
+	// bounds. needm should refresh the stack bounds anyway since this is a
+	// new thread.
+
+	// N.B. since we used a custom stack with makecontext,
+	// callbackUpdateSystemStack had to guess the bounds. Its guess assumes
+	// a 32KiB stack.
+	char *prev_stack_lo = stack2 + STACK_SIZE - (32*1024);
+
+	// New SP is just barely in bounds, but if we don't update the bounds
+	// we'll almost certainly overflow. The SP that
+	// callbackUpdateSystemStack sees already has some data pushed, so it
+	// will be a bit below what we set here. Thus we include some slack.
+	char *new_stack_hi = prev_stack_lo + 128;
+
+	if (getcontext(&uctx_switch) == -1) {
+		perror("getcontext");
+		exit(1);
+	}
+	uctx_switch.uc_stack.ss_sp = new_stack_hi - (STACK_SIZE / 2);
+	uctx_switch.uc_stack.ss_size = STACK_SIZE / 2;
+	uctx_switch.uc_link = &uctx_save;
+	makecontext(&uctx_switch, stackSwitchCallback, 0);
+
+	if (swapcontext(&uctx_save, &uctx_switch) == -1) {
+		perror("swapcontext");
+		exit(1);
+	}
+
 	free(stack2);
 
 	return NULL;
@@ -101,6 +139,9 @@ void callStackSwitchCallbackFromThread(void) {
 	pthread_t thread;
 	assert(pthread_create(&thread, NULL, stackSwitchThread, NULL) == 0);
 	assert(pthread_join(thread, NULL) == 0);
+
+	assert(pthread_create(&thread, NULL, stackSwitchThread2, NULL) == 0);
+	assert(pthread_join(thread, NULL) == 0);
 }
 
 #endif