]> Cypherpunks.ru repositories - gostls13.git/commitdiff
runtime: always keep global reference to mp until mexit completes
authorMichael Pratt <mpratt@google.com>
Tue, 18 Oct 2022 16:01:18 +0000 (12:01 -0400)
committerMichael Pratt <mpratt@google.com>
Tue, 18 Oct 2022 20:57:24 +0000 (20:57 +0000)
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 <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
32 files changed:
src/runtime/os3_solaris.go
src/runtime/os_aix.go
src/runtime/os_js.go
src/runtime/os_openbsd_syscall2.go
src/runtime/os_plan9.go
src/runtime/os_windows.go
src/runtime/proc.go
src/runtime/runtime2.go
src/runtime/stubs2.go
src/runtime/sys_darwin.go
src/runtime/sys_dragonfly_amd64.s
src/runtime/sys_freebsd_386.s
src/runtime/sys_freebsd_amd64.s
src/runtime/sys_freebsd_arm.s
src/runtime/sys_freebsd_arm64.s
src/runtime/sys_freebsd_riscv64.s
src/runtime/sys_linux_386.s
src/runtime/sys_linux_amd64.s
src/runtime/sys_linux_arm.s
src/runtime/sys_linux_arm64.s
src/runtime/sys_linux_loong64.s
src/runtime/sys_linux_mips64x.s
src/runtime/sys_linux_mipsx.s
src/runtime/sys_linux_ppc64x.s
src/runtime/sys_linux_riscv64.s
src/runtime/sys_linux_s390x.s
src/runtime/sys_netbsd_386.s
src/runtime/sys_netbsd_amd64.s
src/runtime/sys_netbsd_arm.s
src/runtime/sys_netbsd_arm64.s
src/runtime/sys_openbsd2.go
src/runtime/sys_openbsd_mips64.s

index 76cf59772bcb050e0e5593d4dedb5f921e76d98b..dcdfe666aca7e9d1524e05b8910cf519ceda5c62 100644 (file)
@@ -7,6 +7,7 @@ package runtime
 import (
        "internal/abi"
        "internal/goarch"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -182,7 +183,7 @@ func newosproc(mp *m) {
        }
 }
 
-func exitThread(wait *uint32) {
+func exitThread(wait *atomic.Uint32) {
        // We should never reach exitThread on Solaris because we let
        // libc clean up threads.
        throw("exitThread")
index 15e4929779a2c5fe4495dc39a22e21ce07add168..104c397e8c050bd07dc1cf784cdacab8d83a4b64 100644 (file)
@@ -8,6 +8,7 @@ package runtime
 
 import (
        "internal/abi"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -233,7 +234,7 @@ func newosproc(mp *m) {
 
 }
 
-func exitThread(wait *uint32) {
+func exitThread(wait *atomic.Uint32) {
        // We should never reach exitThread on AIX because we let
        // libc clean up threads.
        throw("exitThread")
index 7ae0e8d3ece718753fe324677bc9a56123f3e021..7481fb92bf88365618f203b00d3f8b4ea9034e5e 100644 (file)
@@ -7,6 +7,7 @@
 package runtime
 
 import (
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -35,7 +36,7 @@ func usleep_no_g(usec uint32) {
        usleep(usec)
 }
 
-func exitThread(wait *uint32)
+func exitThread(wait *atomic.Uint32)
 
 type mOS struct{}
 
index e4c9d2fe89308430a8c07393272377ef13564f79..ab6b1818285c067cc48330603a733b031e9d7d87 100644 (file)
@@ -7,6 +7,7 @@
 package runtime
 
 import (
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -49,11 +50,11 @@ func open(name *byte, mode, perm int32) int32
 // return value is only set on linux to be used in osinit()
 func madvise(addr unsafe.Pointer, n uintptr, flags int32) int32
 
-// exitThread terminates the current thread, writing *wait = 0 when
+// exitThread terminates the current thread, writing *wait = freeMStack when
 // the stack is safe to reclaim.
 //
 //go:noescape
-func exitThread(wait *uint32)
+func exitThread(wait *atomic.Uint32)
 
 //go:noescape
 func obsdsigprocmask(how int32, new sigset) sigset
index 2836ad9c67f4b40f89f345d4aff4183962ef85c7..5e5a63dcbf97e014ee826656c80af5d918f35ba7 100644 (file)
@@ -468,7 +468,7 @@ func newosproc(mp *m) {
        }
 }
 
-func exitThread(wait *uint32) {
+func exitThread(wait *atomic.Uint32) {
        // We should never reach exitThread on Plan 9 because we let
        // the OS clean up threads.
        throw("exitThread")
index 54261d6fc029cade5acc56f1113f4978e2e2d4c2..44718f1d21459e164f4dcf7f3592160ef9d81059 100644 (file)
@@ -941,7 +941,7 @@ func newosproc0(mp *m, stk unsafe.Pointer) {
        throw("bad newosproc0")
 }
 
-func exitThread(wait *uint32) {
+func exitThread(wait *atomic.Uint32) {
        // We should never reach exitThread on Windows because we let
        // the OS clean up threads.
        throw("exitThread")
index 02390375b5a77ba83645a2efbbb20f58fa73a43b..4285ff6b7c49dcd4a6920c5b5b0fc8ce89a0a69c 100644 (file)
@@ -1582,19 +1582,18 @@ func mexit(osStack bool) {
        }
        throw("m not found in allm")
 found:
-       if !osStack {
-               // Delay reaping m until it's done with the stack.
-               //
-               // If this is using an OS stack, the OS will free it
-               // so there's no need for reaping.
-               atomic.Store(&mp.freeWait, 1)
-               // Put m on the free list, though it will not be reaped until
-               // freeWait is 0. Note that the free list must not be linked
-               // through alllink because some functions walk allm without
-               // locking, so may be using alllink.
-               mp.freelink = sched.freem
-               sched.freem = mp
-       }
+       // Delay reaping m until it's done with the stack.
+       //
+       // Put mp on the free list, though it will not be reaped while freeWait
+       // is freeMWait. mp is no longer reachable via allm, so even if it is
+       // on an OS stack, we must keep a reference to mp alive so that the GC
+       // doesn't free mp while we are still using it.
+       //
+       // Note that the free list must not be linked through alllink because
+       // some functions walk allm without locking, so may be using alllink.
+       mp.freeWait.Store(freeMWait)
+       mp.freelink = sched.freem
+       sched.freem = mp
        unlock(&sched.lock)
 
        atomic.Xadd64(&ncgocall, int64(mp.ncgocall))
@@ -1624,6 +1623,9 @@ found:
        mdestroy(mp)
 
        if osStack {
+               // No more uses of mp, so it is safe to drop the reference.
+               mp.freeWait.Store(freeMRef)
+
                // Return from mstart and let the system thread
                // library free the g0 stack and terminate the thread.
                return
@@ -1795,19 +1797,25 @@ func allocm(pp *p, fn func(), id int64) *m {
                lock(&sched.lock)
                var newList *m
                for freem := sched.freem; freem != nil; {
-                       if freem.freeWait != 0 {
+                       wait := freem.freeWait.Load()
+                       if wait == freeMWait {
                                next := freem.freelink
                                freem.freelink = newList
                                newList = freem
                                freem = next
                                continue
                        }
-                       // stackfree must be on the system stack, but allocm is
-                       // reachable off the system stack transitively from
-                       // startm.
-                       systemstack(func() {
-                               stackfree(freem.g0.stack)
-                       })
+                       // Free the stack if needed. For freeMRef, there is
+                       // nothing to do except drop freem from the sched.freem
+                       // list.
+                       if wait == freeMStack {
+                               // stackfree must be on the system stack, but allocm is
+                               // reachable off the system stack transitively from
+                               // startm.
+                               systemstack(func() {
+                                       stackfree(freem.g0.stack)
+                               })
+                       }
                        freem = freem.freelink
                }
                sched.freem = newList
index 5b55b55ce1c55c397db21e2b8bc56626ebaf9af3..0392f2968fe2d5ec513215c48231349f59c634a1 100644 (file)
@@ -516,6 +516,13 @@ const (
        tlsSize  = tlsSlots * goarch.PtrSize
 )
 
+// Values for m.freeWait.
+const (
+       freeMStack = 0  // M done, free stack and reference.
+       freeMRef   = 1  // M done, free reference.
+       freeMWait  = 2  // M still in use.
+)
+
 type m struct {
        g0      *g     // goroutine with scheduling stack
        morebuf gobuf  // gobuf arg to morestack
@@ -547,7 +554,7 @@ type m struct {
        printlock     int8
        incgo         bool   // m is executing a cgo call
        isextra       bool   // m is an extra m
-       freeWait      uint32 // if == 0, safe to free g0 and delete m (atomic)
+       freeWait      atomic.Uint32 // Whether it is safe to free g0 and delete m (one of freeMRef, freeMStack, freeMWait)
        fastrand      uint64
        needextram    bool
        traceback     uint8
index 94a888dec6289ab10f6958d8c8c201676eaa5fa5..d2ad8d4ec88ed659fa43d02d01924849a6a5ed0c 100644 (file)
@@ -6,7 +6,10 @@
 
 package runtime
 
-import "unsafe"
+import (
+       "runtime/internal/atomic"
+       "unsafe"
+)
 
 // read calls the read system call.
 // 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
 // return value is only set on linux to be used in osinit()
 func madvise(addr unsafe.Pointer, n uintptr, flags int32) int32
 
-// exitThread terminates the current thread, writing *wait = 0 when
+// exitThread terminates the current thread, writing *wait = freeMStack when
 // the stack is safe to reclaim.
 //
 //go:noescape
-func exitThread(wait *uint32)
+func exitThread(wait *atomic.Uint32)
index 1547fdceb0ca0c9e3562f9a30a98bf6b0c027847..88af8944097e258657a08205d556a42c7bc0e99b 100644 (file)
@@ -6,6 +6,7 @@ package runtime
 
 import (
        "internal/abi"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -474,7 +475,7 @@ func pthread_cond_signal(c *pthreadcond) int32 {
 func pthread_cond_signal_trampoline()
 
 // Not used on Darwin, but must be defined.
-func exitThread(wait *uint32) {
+func exitThread(wait *atomic.Uint32) {
 }
 
 //go:nosplit
index 602d5e9b76b4512dce832e2deb74fccae2545e54..0cf98219fb013dffec961b9b72ba4f1a05ba097a 100644 (file)
@@ -65,7 +65,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
        MOVL    $0xf1, 0xf1  // crash
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-8
        MOVQ    wait+0(FP), AX
        // We're done using the stack.
index f919c5a000a5d5469cfdcdfa4b6b1621758c94d0..4e0bc9b08c505164f46be9ccce722fe10935a7b9 100644 (file)
@@ -99,7 +99,7 @@ GLOBL exitStack<>(SB),RODATA,$8
 DATA exitStack<>+0x00(SB)/4, $0
 DATA exitStack<>+0x04(SB)/4, $0
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-4
        MOVL    wait+0(FP), AX
        // We're done using the stack.
index c266d73ea28fdd641d8a12a41c26926f3c9d2d17..374e0ab769ff30ed7e43bea767afe4812b8fa20e 100644 (file)
@@ -96,7 +96,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
        MOVL    $0xf1, 0xf1  // crash
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-8
        MOVQ    wait+0(FP), AX
        // We're done using the stack.
index 89a8d2bfac29a6b92b0b044e3815f130bf1cb1ee..a3fee1426c43ab016264677b886e5e5203ae16a5 100644 (file)
@@ -85,7 +85,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0
        MOVW.CS R8, (R8)
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-4
        MOVW    wait+0(FP), R0
        // We're done using the stack.
index 4f24da62ef162da85a09a4a54ffecc96f3129d12..29866cb4ca4a7d68e92d92083a1c12a340564355 100644 (file)
@@ -99,7 +99,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        MOVD    $0, R0
        MOVD    R0, (R0)
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOVD    wait+0(FP), R0
        // We're done using the stack.
index 3c1b966348dd08748c5030e21ac1e63239519161..4b8699e2bff912636fee0e7545b05aee1739ef05 100644 (file)
@@ -96,7 +96,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        ECALL
        WORD    $0      // crash
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOV     wait+0(FP), A0
        // We're done using the stack.
index 4f5b34b99631f2eba85b835df8200d92c7610ba6..12a294153d2dc0745692ea3ffcb320d651893c8a 100644 (file)
@@ -72,7 +72,7 @@ TEXT exit1<>(SB),NOSPLIT,$0
        INT $3  // not reached
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-4
        MOVL    wait+0(FP), AX
        // We're done using the stack.
index d91f9bd31f13640b044ab15ea16a1bdba45afc73..c7a89ba5363405c7f9903d9bcbfd14f299ff6e02 100644 (file)
@@ -54,7 +54,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
        SYSCALL
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-8
        MOVQ    wait+0(FP), AX
        // We're done using the stack.
index 1bc9e86d6d37df05f30dec0874b410413efcdad1..7b8c4f0e047a4abf53e273c9d8db514822457b42 100644 (file)
@@ -117,7 +117,7 @@ TEXT exit1<>(SB),NOSPLIT|NOFRAME,$0
        MOVW    $1003, R1
        MOVW    R0, (R1)        // fail hard
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-4
        MOVW    wait+0(FP), R0
        // We're done using the stack.
index 04a2cd2da1fcec36d5148e0e3ff64ba5a988196b..38ff6ac33017c006e54669f1d080ff026a54ec6a 100644 (file)
@@ -56,7 +56,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        SVC
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOVD    wait+0(FP), R0
        // We're done using the stack.
index 5705c37496598663961e947806e485301119f194..9ce5e722565f0433aefb8d82c1e9070230c739ad 100644 (file)
@@ -49,7 +49,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        SYSCALL
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOVV    wait+0(FP), R19
        // We're done using the stack.
index 5d113395849b62a3639edeb0ad84c99534b64d1f..47f2da524d1ce65a1d72af368fa607707bf88ca1 100644 (file)
@@ -51,7 +51,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        SYSCALL
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOVV    wait+0(FP), R1
        // We're done using the stack.
index c4507c609885818104b698cb1b65c31efed3290e..5e6b6c150418d2ddfbf846541471553df80dfbfd 100644 (file)
@@ -50,7 +50,7 @@ TEXT runtime·exit(SB),NOSPLIT,$0-4
        UNDEF
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-4
        MOVW    wait+0(FP), R1
        // We're done using the stack.
index 853008d5fe937cfcf75bfb73deab4f2a3586967e..49974c6bf61366cab55b561bdbd5e9b89e022989 100644 (file)
@@ -49,7 +49,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        SYSCALL $SYS_exit_group
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOVD    wait+0(FP), R1
        // We're done using the stack.
index f4fb1c124b2bdeaf9944113372d2c77ee7ab47c9..d1558fd6f799b443b832b20f127fd7bb653800b3 100644 (file)
@@ -57,7 +57,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        ECALL
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOV     wait+0(FP), A0
        // We're done using the stack.
index 777a4747d489d6f913da79bcc62c3dd59518acae..1448670b913b9176040eb4fbcb4754d41d7a88b2 100644 (file)
@@ -46,7 +46,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0-4
        SYSCALL
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT|NOFRAME,$0-8
        MOVD    wait+0(FP), R1
        // We're done using the stack.
index 581b4fc9b689334857a3380d7e0af05b2e0ab945..7be18c61d8b99b0c9e0eb5e3fdb5f6214d5462e3 100644 (file)
@@ -53,7 +53,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-4
        MOVL    $0xf1, 0xf1             // crash
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-4
        MOVL    wait+0(FP), AX
        // We're done using the stack.
index ab11f6ff6690b01ad0ee5f977a278496e74443c3..30f3f380b6fc98566a1c3a61130112a82b5dd238 100644 (file)
@@ -122,7 +122,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
        MOVL    $0xf1, 0xf1             // crash
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-8
        MOVQ    wait+0(FP), AX
        // We're done using the stack.
index dbe3dbcffc0eeb3b8a6f82a412695f49ad7e6588..62fa852adddf3625ccd54b9aee11cf0369122832 100644 (file)
@@ -56,7 +56,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0
        MOVW.CS R8, (R8)
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-4
        MOVW wait+0(FP), R0
        // We're done using the stack.
index fc126cad7d2246ab5436d0c7d0e1f9eacbcdd6b8..d57959f8d76d5559f17b182393eb035db7aca341 100644 (file)
@@ -115,7 +115,7 @@ TEXT runtime·exit(SB),NOSPLIT,$-8
        MOVD    $0, R0                  // If we're still running,
        MOVD    R0, (R0)                // crash
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0-8
        MOVD    wait+0(FP), R0
        // We're done using the stack.
index f936e0cfc3bb771cdf1dc724b6a619af08aa1f8d..f755cd528c8fce370b6ff6249c2776d30aad91f5 100644 (file)
@@ -8,6 +8,7 @@ package runtime
 
 import (
        "internal/abi"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -248,7 +249,7 @@ func sigaltstack(new *stackt, old *stackt) {
 func sigaltstack_trampoline()
 
 // Not used on OpenBSD, but must be defined.
-func exitThread(wait *uint32) {
+func exitThread(wait *atomic.Uint32) {
 }
 
 //go:nosplit
index c2b209205315f2aef71cdaae963df8a2b33854dc..cc37e52e16150ec558770bf59ae6f6dfddf05280 100644 (file)
@@ -24,7 +24,7 @@ TEXT runtime·exit(SB),NOSPLIT|NOFRAME,$0
        MOVV    R2, (R2)
        RET
 
-// func exitThread(wait *uint32)
+// func exitThread(wait *atomic.Uint32)
 TEXT runtime·exitThread(SB),NOSPLIT,$0
        MOVV    wait+0(FP), R4          // arg 1 - notdead
        MOVV    $302, R2                // sys___threxit