Skip to content

Commit e252dcf

Browse files
prattmicprattmic
committed
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. Fixes #56243. 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]>
1 parent a8e4b8c commit e252dcf

32 files changed

+78
-54
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
@@ -468,7 +468,7 @@ func newosproc(mp *m) {
468468
}
469469
}
470470

471-
func exitThread(wait *uint32) {
471+
func exitThread(wait *atomic.Uint32) {
472472
// We should never reach exitThread on Plan 9 because we let
473473
// the OS clean up threads.
474474
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
@@ -1582,19 +1582,18 @@ func mexit(osStack bool) {
15821582
}
15831583
throw("m not found in allm")
15841584
found:
1585-
if !osStack {
1586-
// Delay reaping m until it's done with the stack.
1587-
//
1588-
// If this is using an OS stack, the OS will free it
1589-
// so there's no need for reaping.
1590-
atomic.Store(&mp.freeWait, 1)
1591-
// Put m on the free list, though it will not be reaped until
1592-
// freeWait is 0. Note that the free list must not be linked
1593-
// through alllink because some functions walk allm without
1594-
// locking, so may be using alllink.
1595-
mp.freelink = sched.freem
1596-
sched.freem = mp
1597-
}
1585+
// Delay reaping m until it's done with the stack.
1586+
//
1587+
// Put mp on the free list, though it will not be reaped while freeWait
1588+
// is freeMWait. mp is no longer reachable via allm, so even if it is
1589+
// on an OS stack, we must keep a reference to mp alive so that the GC
1590+
// doesn't free mp while we are still using it.
1591+
//
1592+
// Note that the free list must not be linked through alllink because
1593+
// some functions walk allm without locking, so may be using alllink.
1594+
mp.freeWait.Store(freeMWait)
1595+
mp.freelink = sched.freem
1596+
sched.freem = mp
15981597
unlock(&sched.lock)
15991598

16001599
atomic.Xadd64(&ncgocall, int64(mp.ncgocall))
@@ -1624,6 +1623,9 @@ found:
16241623
mdestroy(mp)
16251624

16261625
if osStack {
1626+
// No more uses of mp, so it is safe to drop the reference.
1627+
mp.freeWait.Store(freeMRef)
1628+
16271629
// Return from mstart and let the system thread
16281630
// library free the g0 stack and terminate the thread.
16291631
return
@@ -1795,19 +1797,25 @@ func allocm(pp *p, fn func(), id int64) *m {
17951797
lock(&sched.lock)
17961798
var newList *m
17971799
for freem := sched.freem; freem != nil; {
1798-
if freem.freeWait != 0 {
1800+
wait := freem.freeWait.Load()
1801+
if wait == freeMWait {
17991802
next := freem.freelink
18001803
freem.freelink = newList
18011804
newList = freem
18021805
freem = next
18031806
continue
18041807
}
1805-
// stackfree must be on the system stack, but allocm is
1806-
// reachable off the system stack transitively from
1807-
// startm.
1808-
systemstack(func() {
1809-
stackfree(freem.g0.stack)
1810-
})
1808+
// Free the stack if needed. For freeMRef, there is
1809+
// nothing to do except drop freem from the sched.freem
1810+
// list.
1811+
if wait == freeMStack {
1812+
// stackfree must be on the system stack, but allocm is
1813+
// reachable off the system stack transitively from
1814+
// startm.
1815+
systemstack(func() {
1816+
stackfree(freem.g0.stack)
1817+
})
1818+
}
18111819
freem = freem.freelink
18121820
}
18131821
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
@@ -547,7 +554,7 @@ type m struct {
547554
printlock int8
548555
incgo bool // m is executing a cgo call
549556
isextra bool // m is an extra m
550-
freeWait uint32 // if == 0, safe to free g0 and delete m (atomic)
557+
freeWait atomic.Uint32 // Whether it is safe to free g0 and delete m (one of freeMRef, freeMStack, freeMWait)
551558
fastrand uint64
552559
needextram bool
553560
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
@@ -99,7 +99,7 @@ GLOBL exitStack<>(SB),RODATA,$8
9999
DATA exitStack<>+0x00(SB)/4, $0
100100
DATA exitStack<>+0x04(SB)/4, $0
101101

102-
// func exitThread(wait *uint32)
102+
// func exitThread(wait *atomic.Uint32)
103103
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
104104
MOVL wait+0(FP), AX
105105
// 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
@@ -96,7 +96,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
9696
MOVL $0xf1, 0xf1 // crash
9797
RET
9898

99-
// func exitThread(wait *uint32)
99+
// func exitThread(wait *atomic.uint32)
100100
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
101101
MOVQ wait+0(FP), AX
102102
// 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_freebsd_riscv64.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
9696
ECALL
9797
WORD $0 // crash
9898

99-
// func exitThread(wait *uint32)
99+
// func exitThread(wait *atomic.Uint32)
100100
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
101101
MOV wait+0(FP), A0
102102
// 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
@@ -72,7 +72,7 @@ TEXT exit1<>(SB),NOSPLIT,$0
7272
INT $3 // not reached
7373
RET
7474

75-
// func exitThread(wait *uint32)
75+
// func exitThread(wait *atomic.Uint32)
7676
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
7777
MOVL wait+0(FP), AX
7878
// 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
@@ -54,7 +54,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
5454
SYSCALL
5555
RET
5656

57-
// func exitThread(wait *uint32)
57+
// func exitThread(wait *atomic.Uint32)
5858
TEXT runtime·exitThread(SB),NOSPLIT,$0-8
5959
MOVQ wait+0(FP), AX
6060
// 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
@@ -117,7 +117,7 @@ TEXT exit1<>(SB),NOSPLIT|NOFRAME,$0
117117
MOVW $1003, R1
118118
MOVW R0, (R1) // fail hard
119119

120-
// func exitThread(wait *uint32)
120+
// func exitThread(wait *atomic.Uint32)
121121
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-4
122122
MOVW wait+0(FP), R0
123123
// 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
@@ -56,7 +56,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5656
SVC
5757
RET
5858

59-
// func exitThread(wait *uint32)
59+
// func exitThread(wait *atomic.Uint32)
6060
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
6161
MOVD wait+0(FP), R0
6262
// 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
@@ -49,7 +49,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
4949
SYSCALL
5050
RET
5151

52-
// func exitThread(wait *uint32)
52+
// func exitThread(wait *atomic.Uint32)
5353
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5454
MOVV wait+0(FP), R19
5555
// 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
@@ -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
MOVV wait+0(FP), R1
5757
// 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
@@ -50,7 +50,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
5050
UNDEF
5151
RET
5252

53-
// func exitThread(wait *uint32)
53+
// func exitThread(wait *atomic.Uint32)
5454
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
5555
MOVW wait+0(FP), R1
5656
// 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
@@ -49,7 +49,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
4949
SYSCALL $SYS_exit_group
5050
RET
5151

52-
// func exitThread(wait *uint32)
52+
// func exitThread(wait *atomic.Uint32)
5353
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5454
MOVD wait+0(FP), R1
5555
// 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
@@ -57,7 +57,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5757
ECALL
5858
RET
5959

60-
// func exitThread(wait *uint32)
60+
// func exitThread(wait *atomic.Uint32)
6161
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
6262
MOV wait+0(FP), A0
6363
// 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
@@ -46,7 +46,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
4646
SYSCALL
4747
RET
4848

49-
// func exitThread(wait *uint32)
49+
// func exitThread(wait *atomic.Uint32)
5050
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5151
MOVD wait+0(FP), R1
5252
// 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.

0 commit comments

Comments
 (0)