]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.19] 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)
committerHeschi Kreinick <heschi@google.com>
Mon, 24 Oct 2022 17:28:39 +0000 (17:28 +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.

For #56243.
Fixes #56309.

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>
(cherry picked from commit e252dcf9d38ce9192bccacb7e33867cbfbd22b6c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/443815
Reviewed-by: Austin Clements <austin@google.com>
31 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_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 8c85b71532dfd927a488a5c70ac7971e8681b44a..79f6b18c9ae497fe5a62f2b25e80241452343d50 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 34cc0271f050b70a77549016cad91ce770b87b3a..718e91e1d40fecd1b32b417ab45b8ac80465a0fa 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 f0e7c6ae70f814e28562182af88604e20377641f..1e8ef9c67bbd5d4e9b05aab89d23da916358fdba 100644 (file)
@@ -461,7 +461,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 2f6ec75cf89646fc0c974b5a0344caa7444457c9..1ad7c8b790650f4fbd4c45ccafa5236ce92585bb 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 3991a48b10482ddeca5f562d4bf861856b51ba63..08fe441a0cd033f16ac8e70756551741c3a0b58c 100644 (file)
@@ -1516,19 +1516,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(&m.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.
-               m.freelink = sched.freem
-               sched.freem = m
-       }
+       // 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.
+       m.freeWait.Store(freeMWait)
+       m.freelink = sched.freem
+       sched.freem = m
        unlock(&sched.lock)
 
        atomic.Xadd64(&ncgocall, int64(m.ncgocall))
@@ -1558,6 +1557,9 @@ found:
        mdestroy(m)
 
        if osStack {
+               // No more uses of mp, so it is safe to drop the reference.
+               m.freeWait.Store(freeMRef)
+
                // Return from mstart and let the system thread
                // library free the g0 stack and terminate the thread.
                return
@@ -1729,19 +1731,25 @@ func allocm(_p_ *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 e1788223e7ee630adcfde72a199009768424ba34..2a42809f0d9a6f546d6c68eb7d02d7af521c3984 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
@@ -546,7 +553,7 @@ type m struct {
        newSigstack   bool // minit on C thread called sigaltstack
        printlock     int8
        incgo         bool   // m is executing a cgo call
-       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 9e5210b0db71ce17ca469465f436b290d4bd0b0b..d10eb3fa4b25e9749b78cb75cb66478f7ed7e334 100644 (file)
@@ -63,7 +63,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 94341f6d4e456ba4bd9f49b076a1d470b6e53ded..9f48d585145fc9964660ee472d9a33cbd70c5293 100644 (file)
@@ -60,7 +60,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 cbee34d13be7f74770e40024b306b6c29c167d8f..a6099b465cc3147d9d38f69b0b91b0d49a7aec1f 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 5dcdf375ab5c63afa96f3ad10741cbdafa5a6174..4143a8f1eb40ee1c30e97e76f3bfe886ba35c619 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 4942f21e4f430c93a9c1e5c6ad9f20a6087487bf..e7d9198831eaff1f3c3871fd4f56757d00719128 100644 (file)
@@ -77,7 +77,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 ca6ecb13eb7e06d98316e2c715445b46ce655ef0..242517a86358b190e07747b940e8486976fd69b2 100644 (file)
@@ -59,7 +59,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 66bf40328ebbdb2afecbee871d8dfa4aae2fd7ce..b0250936f40c7afa380d450ec0e555f9186ae6c3 100644 (file)
@@ -122,7 +122,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 b47b6fd0a05f6d857deb78e38a4a97facd28010e..7cfd818efc5258a389a80c9d6fee842d30ec9fa5 100644 (file)
@@ -60,7 +60,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 36a92df87cf6a55a8c483cde9b5038fa23109910..1ebc77bff363d15a67ab1c578da02e9debb17d36 100644 (file)
@@ -53,7 +53,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 06d54dff78f0c60df0fb9c8bbd382f70e725b1ee..e38f444e28b7a5fab65ea6b7b54a1a73d387f103 100644 (file)
@@ -56,7 +56,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 e70edcc0e22fdc017b2c6e770dcb026f78f4a7f8..8f958193b66123290e67fada7d8c495142bfe9ab 100644 (file)
@@ -55,7 +55,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 2913a05f56f86e9f2670a07a7a841cbe48f49125..6e9d8f56f1dd035b9f94223a62bf15fb6031668d 100644 (file)
@@ -54,7 +54,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 afb2d11da98fdc6af4abdac1e0f3a4b228fd502f..8d922b33446cc8434cb6875f265ce4aa7456cc66 100644 (file)
@@ -61,7 +61,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 c82cb9b4aab12634de4e39d607b4a66f5bd35c5d..a5fde911e634a87de4f491aff7807dcca43d63e2 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
        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