Skip to content

Commit 0cc20ec

Browse files
prattmicheschi
prattmic
authored and
heschi
committed
[release-branch.go1.19] runtime: always keep global reference to mp until mexit completes
Ms are allocated via standard heap allocation (`new(m)`), which means we must keep them alive (i.e., reachable by the GC) until we are completely done using them. Ms are primarily reachable through runtime.allm. However, runtime.mexit drops the M from allm fairly early, long before it is done using the M structure. If that was the last reference to the M, it is now at risk of being freed by the GC and used for some other allocation, leading to memory corruption. Ms with a Go-allocated stack coincidentally already keep a reference to the M in sched.freem, so that the stack can be freed lazily. This reference has the side effect of keeping this Ms reachable. However, Ms with an OS stack skip this and are at risk of corruption. Fix this lifetime by extending sched.freem use to all Ms, with the value of mp.freeWait determining whether the stack needs to be freed or not. For #56243. Fixes #56309. Change-Id: Ic0c01684775f5646970df507111c9abaac0ba52e Reviewed-on: https://go-review.googlesource.com/c/go/+/443716 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Michael Pratt <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> (cherry picked from commit e252dcf) Reviewed-on: https://go-review.googlesource.com/c/go/+/443815 Reviewed-by: Austin Clements <[email protected]>
1 parent 8d10cc0 commit 0cc20ec

31 files changed

+77
-53
lines changed

‎src/runtime/os3_solaris.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package runtime
77
import (
88
"internal/abi"
99
"internal/goarch"
10+
"runtime/internal/atomic"
1011
"unsafe"
1112
)
1213

@@ -182,7 +183,7 @@ func newosproc(mp *m) {
182183
}
183184
}
184185

185-
func exitThread(wait *uint32) {
186+
func exitThread(wait *atomic.Uint32) {
186187
// We should never reach exitThread on Solaris because we let
187188
// libc clean up threads.
188189
throw("exitThread")

‎src/runtime/os_aix.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package runtime
88

99
import (
1010
"internal/abi"
11+
"runtime/internal/atomic"
1112
"unsafe"
1213
)
1314

@@ -233,7 +234,7 @@ func newosproc(mp *m) {
233234

234235
}
235236

236-
func exitThread(wait *uint32) {
237+
func exitThread(wait *atomic.Uint32) {
237238
// We should never reach exitThread on AIX because we let
238239
// libc clean up threads.
239240
throw("exitThread")

‎src/runtime/os_js.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package runtime
88

99
import (
10+
"runtime/internal/atomic"
1011
"unsafe"
1112
)
1213

@@ -35,7 +36,7 @@ func usleep_no_g(usec uint32) {
3536
usleep(usec)
3637
}
3738

38-
func exitThread(wait *uint32)
39+
func exitThread(wait *atomic.Uint32)
3940

4041
type mOS struct{}
4142

‎src/runtime/os_openbsd_syscall2.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package runtime
88

99
import (
10+
"runtime/internal/atomic"
1011
"unsafe"
1112
)
1213

@@ -49,11 +50,11 @@ func open(name *byte, mode, perm int32) int32
4950
// return value is only set on linux to be used in osinit()
5051
func madvise(addr unsafe.Pointer, n uintptr, flags int32) int32
5152

52-
// exitThread terminates the current thread, writing *wait = 0 when
53+
// exitThread terminates the current thread, writing *wait = freeMStack when
5354
// the stack is safe to reclaim.
5455
//
5556
//go:noescape
56-
func exitThread(wait *uint32)
57+
func exitThread(wait *atomic.Uint32)
5758

5859
//go:noescape
5960
func obsdsigprocmask(how int32, new sigset) sigset

‎src/runtime/os_plan9.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ func newosproc(mp *m) {
461461
}
462462
}
463463

464-
func exitThread(wait *uint32) {
464+
func exitThread(wait *atomic.Uint32) {
465465
// We should never reach exitThread on Plan 9 because we let
466466
// the OS clean up threads.
467467
throw("exitThread")

‎src/runtime/os_windows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -941,7 +941,7 @@ func newosproc0(mp *m, stk unsafe.Pointer) {
941941
throw("bad newosproc0")
942942
}
943943

944-
func exitThread(wait *uint32) {
944+
func exitThread(wait *atomic.Uint32) {
945945
// We should never reach exitThread on Windows because we let
946946
// the OS clean up threads.
947947
throw("exitThread")

‎src/runtime/proc.go

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,19 +1516,18 @@ func mexit(osStack bool) {
15161516
}
15171517
throw("m not found in allm")
15181518
found:
1519-
if !osStack {
1520-
// Delay reaping m until it's done with the stack.
1521-
//
1522-
// If this is using an OS stack, the OS will free it
1523-
// so there's no need for reaping.
1524-
atomic.Store(&m.freeWait, 1)
1525-
// Put m on the free list, though it will not be reaped until
1526-
// freeWait is 0. Note that the free list must not be linked
1527-
// through alllink because some functions walk allm without
1528-
// locking, so may be using alllink.
1529-
m.freelink = sched.freem
1530-
sched.freem = m
1531-
}
1519+
// Delay reaping m until it's done with the stack.
1520+
//
1521+
// Put mp on the free list, though it will not be reaped while freeWait
1522+
// is freeMWait. mp is no longer reachable via allm, so even if it is
1523+
// on an OS stack, we must keep a reference to mp alive so that the GC
1524+
// doesn't free mp while we are still using it.
1525+
//
1526+
// Note that the free list must not be linked through alllink because
1527+
// some functions walk allm without locking, so may be using alllink.
1528+
m.freeWait.Store(freeMWait)
1529+
m.freelink = sched.freem
1530+
sched.freem = m
15321531
unlock(&sched.lock)
15331532

15341533
atomic.Xadd64(&ncgocall, int64(m.ncgocall))
@@ -1558,6 +1557,9 @@ found:
15581557
mdestroy(m)
15591558

15601559
if osStack {
1560+
// No more uses of mp, so it is safe to drop the reference.
1561+
m.freeWait.Store(freeMRef)
1562+
15611563
// Return from mstart and let the system thread
15621564
// library free the g0 stack and terminate the thread.
15631565
return
@@ -1729,19 +1731,25 @@ func allocm(_p_ *p, fn func(), id int64) *m {
17291731
lock(&sched.lock)
17301732
var newList *m
17311733
for freem := sched.freem; freem != nil; {
1732-
if freem.freeWait != 0 {
1734+
wait := freem.freeWait.Load()
1735+
if wait == freeMWait {
17331736
next := freem.freelink
17341737
freem.freelink = newList
17351738
newList = freem
17361739
freem = next
17371740
continue
17381741
}
1739-
// stackfree must be on the system stack, but allocm is
1740-
// reachable off the system stack transitively from
1741-
// startm.
1742-
systemstack(func() {
1743-
stackfree(freem.g0.stack)
1744-
})
1742+
// Free the stack if needed. For freeMRef, there is
1743+
// nothing to do except drop freem from the sched.freem
1744+
// list.
1745+
if wait == freeMStack {
1746+
// stackfree must be on the system stack, but allocm is
1747+
// reachable off the system stack transitively from
1748+
// startm.
1749+
systemstack(func() {
1750+
stackfree(freem.g0.stack)
1751+
})
1752+
}
17451753
freem = freem.freelink
17461754
}
17471755
sched.freem = newList

‎src/runtime/runtime2.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,13 @@ const (
516516
tlsSize = tlsSlots * goarch.PtrSize
517517
)
518518

519+
// Values for m.freeWait.
520+
const (
521+
freeMStack = 0 // M done, free stack and reference.
522+
freeMRef = 1 // M done, free reference.
523+
freeMWait = 2 // M still in use.
524+
)
525+
519526
type m struct {
520527
g0 *g // goroutine with scheduling stack
521528
morebuf gobuf // gobuf arg to morestack
@@ -546,7 +553,7 @@ type m struct {
546553
newSigstack bool // minit on C thread called sigaltstack
547554
printlock int8
548555
incgo bool // m is executing a cgo call
549-
freeWait uint32 // if == 0, safe to free g0 and delete m (atomic)
556+
freeWait atomic.Uint32 // Whether it is safe to free g0 and delete m (one of freeMRef, freeMStack, freeMWait)
550557
fastrand uint64
551558
needextram bool
552559
traceback uint8

‎src/runtime/stubs2.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66

77
package runtime
88

9-
import "unsafe"
9+
import (
10+
"runtime/internal/atomic"
11+
"unsafe"
12+
)
1013

1114
// read calls the read system call.
1215
// It returns a non-negative number of bytes written or a negative errno value.
@@ -34,8 +37,8 @@ func open(name *byte, mode, perm int32) int32
3437
// return value is only set on linux to be used in osinit()
3538
func madvise(addr unsafe.Pointer, n uintptr, flags int32) int32
3639

37-
// exitThread terminates the current thread, writing *wait = 0 when
40+
// exitThread terminates the current thread, writing *wait = freeMStack when
3841
// the stack is safe to reclaim.
3942
//
4043
//go:noescape
41-
func exitThread(wait *uint32)
44+
func exitThread(wait *atomic.Uint32)

‎src/runtime/sys_darwin.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package runtime
66

77
import (
88
"internal/abi"
9+
"runtime/internal/atomic"
910
"unsafe"
1011
)
1112

@@ -474,7 +475,7 @@ func pthread_cond_signal(c *pthreadcond) int32 {
474475
func pthread_cond_signal_trampoline()
475476

476477
// Not used on Darwin, but must be defined.
477-
func exitThread(wait *uint32) {
478+
func exitThread(wait *atomic.Uint32) {
478479
}
479480

480481
//go:nosplit

‎src/runtime/sys_dragonfly_amd64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
6565
MOVL $0xf1, 0xf1 // crash
6666
RET
6767

68-
// func exitThread(wait *uint32)
68+
// func exitThread(wait *atomic.Uint32)
6969
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
7070
MOVQ wait+0(FP), AX
7171
// We're done using the stack.

‎src/runtime/sys_freebsd_386.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ GLOBL exitStack<>(SB),RODATA,$8
6363
DATA exitStack<>+0x00(SB)/4, $0
6464
DATA exitStack<>+0x04(SB)/4, $0
6565

66-
// func exitThread(wait *uint32)
66+
// func exitThread(wait *atomic.Uint32)
6767
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
6868
MOVL wait+0(FP), AX
6969
// We're done using the stack.

‎src/runtime/sys_freebsd_amd64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
6060
MOVL $0xf1, 0xf1 // crash
6161
RET
6262

63-
// func exitThread(wait *uint32)
63+
// func exitThread(wait *atomic.uint32)
6464
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
6565
MOVQ wait+0(FP), AX
6666
// We're done using the stack.

‎src/runtime/sys_freebsd_arm.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0
8585
MOVW.CS R8, (R8)
8686
RET
8787

88-
// func exitThread(wait *uint32)
88+
// func exitThread(wait *atomic.Uint32)
8989
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
9090
MOVW wait+0(FP), R0
9191
// We're done using the stack.

‎src/runtime/sys_freebsd_arm64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
9999
MOVD $0, R0
100100
MOVD R0, (R0)
101101

102-
// func exitThread(wait *uint32)
102+
// func exitThread(wait *atomic.Uint32)
103103
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
104104
MOVD wait+0(FP), R0
105105
// We're done using the stack.

‎src/runtime/sys_linux_386.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ TEXT exit1<>(SB),NOSPLIT,$0
7777
INT $3 // not reached
7878
RET
7979

80-
// func exitThread(wait *uint32)
80+
// func exitThread(wait *atomic.Uint32)
8181
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
8282
MOVL wait+0(FP), AX
8383
// We're done using the stack.

‎src/runtime/sys_linux_amd64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
5959
SYSCALL
6060
RET
6161

62-
// func exitThread(wait *uint32)
62+
// func exitThread(wait *atomic.Uint32)
6363
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
6464
MOVQ wait+0(FP), AX
6565
// We're done using the stack.

‎src/runtime/sys_linux_arm.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ TEXT exit1<>(SB),NOSPLIT|NOFRAME,$0
122122
MOVW $1003, R1
123123
MOVW R0, (R1) // fail hard
124124

125-
// func exitThread(wait *uint32)
125+
// func exitThread(wait *atomic.Uint32)
126126
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-4
127127
MOVW wait+0(FP), R0
128128
// We're done using the stack.

‎src/runtime/sys_linux_arm64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
6060
SVC
6161
RET
6262

63-
// func exitThread(wait *uint32)
63+
// func exitThread(wait *atomic.Uint32)
6464
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
6565
MOVD wait+0(FP), R0
6666
// We're done using the stack.

‎src/runtime/sys_linux_loong64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5353
SYSCALL
5454
RET
5555

56-
// func exitThread(wait *uint32)
56+
// func exitThread(wait *atomic.Uint32)
5757
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5858
MOVV wait+0(FP), R19
5959
// We're done using the stack.

‎src/runtime/sys_linux_mips64x.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5656
SYSCALL
5757
RET
5858

59-
// func exitThread(wait *uint32)
59+
// func exitThread(wait *atomic.Uint32)
6060
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
6161
MOVV wait+0(FP), R1
6262
// We're done using the stack.

‎src/runtime/sys_linux_mipsx.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
5555
UNDEF
5656
RET
5757

58-
// func exitThread(wait *uint32)
58+
// func exitThread(wait *atomic.Uint32)
5959
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
6060
MOVW wait+0(FP), R1
6161
// We're done using the stack.

‎src/runtime/sys_linux_ppc64x.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5454
SYSCALL $SYS_exit_group
5555
RET
5656

57-
// func exitThread(wait *uint32)
57+
// func exitThread(wait *atomic.Uint32)
5858
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5959
MOVD wait+0(FP), R1
6060
// We're done using the stack.

‎src/runtime/sys_linux_riscv64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
6161
ECALL
6262
RET
6363

64-
// func exitThread(wait *uint32)
64+
// func exitThread(wait *atomic.Uint32)
6565
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
6666
MOV wait+0(FP), A0
6767
// We're done using the stack.

‎src/runtime/sys_linux_s390x.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5151
SYSCALL
5252
RET
5353

54-
// func exitThread(wait *uint32)
54+
// func exitThread(wait *atomic.Uint32)
5555
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5656
MOVD wait+0(FP), R1
5757
// We're done using the stack.

‎src/runtime/sys_netbsd_386.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-4
5353
MOVL $0xf1, 0xf1 // crash
5454
RET
5555

56-
// func exitThread(wait *uint32)
56+
// func exitThread(wait *atomic.Uint32)
5757
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
5858
MOVL wait+0(FP), AX
5959
// We're done using the stack.

‎src/runtime/sys_netbsd_amd64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
122122
MOVL $0xf1, 0xf1 // crash
123123
RET
124124

125-
// func exitThread(wait *uint32)
125+
// func exitThread(wait *atomic.Uint32)
126126
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
127127
MOVQ wait+0(FP), AX
128128
// We're done using the stack.

‎src/runtime/sys_netbsd_arm.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0
5656
MOVW.CS R8, (R8)
5757
RET
5858

59-
// func exitThread(wait *uint32)
59+
// func exitThread(wait *atomic.Uint32)
6060
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
6161
MOVW wait+0(FP), R0
6262
// We're done using the stack.

‎src/runtime/sys_netbsd_arm64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
115115
MOVD $0, R0 // If we're still running,
116116
MOVD R0, (R0) // crash
117117

118-
// func exitThread(wait *uint32)
118+
// func exitThread(wait *atomic.Uint32)
119119
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
120120
MOVD wait+0(FP), R0
121121
// We're done using the stack.

0 commit comments

Comments
 (0)