Skip to content

Commit 2c2952a

Browse files
prattmicheschi
prattmic
authored and
heschi
committed
[release-branch.go1.18] 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 #56308. 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/+/443816 Reviewed-by: Austin Clements <[email protected]>
1 parent 828be9a commit 2c2952a

30 files changed

+76
-52
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

@@ -184,7 +185,7 @@ func newosproc(mp *m) {
184185
}
185186
}
186187

187-
func exitThread(wait *uint32) {
188+
func exitThread(wait *atomic.Uint32) {
188189
// We should never reach exitThread on Solaris because we let
189190
// libc clean up threads.
190191
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

@@ -232,7 +233,7 @@ func newosproc(mp *m) {
232233

233234
}
234235

235-
func exitThread(wait *uint32) {
236+
func exitThread(wait *atomic.Uint32) {
236237
// We should never reach exitThread on AIX because we let
237238
// libc clean up threads.
238239
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

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

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

5758
//go:noescape
5859
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
@@ -458,7 +458,7 @@ func newosproc(mp *m) {
458458
}
459459
}
460460

461-
func exitThread(wait *uint32) {
461+
func exitThread(wait *atomic.Uint32) {
462462
// We should never reach exitThread on Plan 9 because we let
463463
// the OS clean up threads.
464464
throw("exitThread")

‎src/runtime/os_windows.go

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

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

‎src/runtime/proc.go

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

15261525
atomic.Xadd64(&ncgocall, int64(m.ncgocall))
@@ -1550,6 +1549,9 @@ found:
15501549
mdestroy(m)
15511550

15521551
if osStack {
1552+
// No more uses of mp, so it is safe to drop the reference.
1553+
m.freeWait.Store(freeMRef)
1554+
15531555
// Return from mstart and let the system thread
15541556
// library free the g0 stack and terminate the thread.
15551557
return
@@ -1721,19 +1723,25 @@ func allocm(_p_ *p, fn func(), id int64) *m {
17211723
lock(&sched.lock)
17221724
var newList *m
17231725
for freem := sched.freem; freem != nil; {
1724-
if freem.freeWait != 0 {
1726+
wait := freem.freeWait.Load()
1727+
if wait == freeMWait {
17251728
next := freem.freelink
17261729
freem.freelink = newList
17271730
newList = freem
17281731
freem = next
17291732
continue
17301733
}
1731-
// stackfree must be on the system stack, but allocm is
1732-
// reachable off the system stack transitively from
1733-
// startm.
1734-
systemstack(func() {
1735-
stackfree(freem.g0.stack)
1736-
})
1734+
// Free the stack if needed. For freeMRef, there is
1735+
// nothing to do except drop freem from the sched.freem
1736+
// list.
1737+
if wait == freeMStack {
1738+
// stackfree must be on the system stack, but allocm is
1739+
// reachable off the system stack transitively from
1740+
// startm.
1741+
systemstack(func() {
1742+
stackfree(freem.g0.stack)
1743+
})
1744+
}
17371745
freem = freem.freelink
17381746
}
17391747
sched.freem = newList

‎src/runtime/runtime2.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,13 @@ const (
510510
tlsSize = tlsSlots * goarch.PtrSize
511511
)
512512

513+
// Values for m.freeWait.
514+
const (
515+
freeMStack = 0 // M done, free stack and reference.
516+
freeMRef = 1 // M done, free reference.
517+
freeMWait = 2 // M still in use.
518+
)
519+
513520
type m struct {
514521
g0 *g // goroutine with scheduling stack
515522
morebuf gobuf // gobuf arg to morestack
@@ -540,7 +547,7 @@ type m struct {
540547
newSigstack bool // minit on C thread called sigaltstack
541548
printlock int8
542549
incgo bool // m is executing a cgo call
543-
freeWait uint32 // if == 0, safe to free g0 and delete m (atomic)
550+
freeWait atomic.Uint32 // Whether it is safe to free g0 and delete m (one of freeMRef, freeMStack, freeMWait)
544551
fastrand uint64
545552
needextram bool
546553
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.
@@ -33,8 +36,8 @@ func open(name *byte, mode, perm int32) int32
3336
// return value is only set on linux to be used in osinit()
3437
func madvise(addr unsafe.Pointer, n uintptr, flags int32) int32
3538

36-
// exitThread terminates the current thread, writing *wait = 0 when
39+
// exitThread terminates the current thread, writing *wait = freeMStack when
3740
// the stack is safe to reclaim.
3841
//
3942
//go:noescape
40-
func exitThread(wait *uint32)
43+
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

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

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

479480
//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
@@ -86,7 +86,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0
8686
MOVW.CS R8, (R8)
8787
RET
8888

89-
// func exitThread(wait *uint32)
89+
// func exitThread(wait *atomic.Uint32)
9090
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
9191
MOVW wait+0(FP), R0
9292
// 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
@@ -98,7 +98,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
9898
MOVD $0, R0
9999
MOVD R0, (R0)
100100

101-
// func exitThread(wait *uint32)
101+
// func exitThread(wait *atomic.Uint32)
102102
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
103103
MOVD wait+0(FP), R0
104104
// 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
@@ -78,7 +78,7 @@ TEXT exit1<>(SB),NOSPLIT,$0
7878
INT $3 // not reached
7979
RET
8080

81-
// func exitThread(wait *uint32)
81+
// func exitThread(wait *atomic.Uint32)
8282
TEXT runtime·exitThread(SB),NOSPLIT,$0-4
8383
MOVL wait+0(FP), AX
8484
// 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
@@ -60,7 +60,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
6060
SYSCALL
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_linux_arm.s

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

134-
// func exitThread(wait *uint32)
134+
// func exitThread(wait *atomic.Uint32)
135135
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-4
136136
MOVW wait+0(FP), R0
137137
// 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
@@ -59,7 +59,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5959
SVC
6060
RET
6161

62-
// func exitThread(wait *uint32)
62+
// func exitThread(wait *atomic.Uint32)
6363
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
6464
MOVD wait+0(FP), R0
6565
// 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
@@ -56,7 +56,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
5656
UNDEF
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), R1
6262
// 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
@@ -55,7 +55,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
5555
SYSCALL $SYS_exit_group
5656
RET
5757

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

55-
// func exitThread(wait *uint32)
55+
// func exitThread(wait *atomic.Uint32)
5656
TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
5757
MOVD wait+0(FP), R1
5858
// 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
@@ -114,7 +114,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
114114
MOVD $0, R0 // If we're still running,
115115
MOVD R0, (R0) // crash
116116

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

0 commit comments

Comments
 (0)