]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.18] 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:47 +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 #56308.

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/+/443816
Reviewed-by: Austin Clements <austin@google.com>
30 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_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 4aba0ff64ba3da16d3ac78b21e2fe6940a9454fe..3ccce776ca35d64c7bb58762ec248fda784d056b 100644 (file)
@@ -7,6 +7,7 @@ package runtime
 import (
        "internal/abi"
        "internal/goarch"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -184,7 +185,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 292ff94795677a65c8c661867a7f118e742e1d96..41352b3a5a1f19109f31adb128e9a2ce977d46a7 100644 (file)
@@ -8,6 +8,7 @@ package runtime
 
 import (
        "internal/abi"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -232,7 +233,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 9ed916705bc517e906f1f1618fc0e58ae3492277..6f358d36d6f1621d15c4da2b9687aef0085bc2dc 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 99542fb2de161786d8974b17fb96d3476205b99e..810d599508886f8ba555ee0ee3635c0053492524 100644 (file)
@@ -7,6 +7,7 @@
 package runtime
 
 import (
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -48,11 +49,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 975d460a7da924ede62917c0b6800d826cfdf4c5..2bb6ec0fcee7e8a2f2ce22e742afd998fa2f938a 100644 (file)
@@ -458,7 +458,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 c76add78025a26bb72455dc4a8728f449ade88ed..b49e9baec5880302871fcc34770c0a85e39be6d8 100644 (file)
@@ -939,7 +939,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 b997a467bab447b58edd9475504a6ab0de8a5614..cae15bc8e2c32d6df7f53f2bded6fa742e845c76 100644 (file)
@@ -1508,19 +1508,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))
@@ -1550,6 +1549,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
@@ -1721,19 +1723,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 b40045e4a5b36e399192068c52e6881807062401..9cd4777cf8de2d7e8ebdc038064a6fad639a9893 100644 (file)
@@ -510,6 +510,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
@@ -540,7 +547,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 9aa965454d991a4fda9dd84c786d65f5d9f94f8d..0d211e2db98e44609b381c374375c1fae0a37d78 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.
@@ -33,8 +36,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 58b3a9171c79740c98b786eb0af32dbed618d723..c90cc78968caca7e79888e1e0632fdf8a2112404 100644 (file)
@@ -6,6 +6,7 @@ package runtime
 
 import (
        "internal/abi"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -473,7 +474,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 d57bc2a7a4ee8591a5e2c58cde77772668f6aaa0..3af0928828a7af1bd08cc46519311b16eb91e699 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 97e6d9ab36634ced64091e5831d2d4589405824d..d4c4cc7fdbbd076ec65a18eafae84d5a6d5d85f6 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 165e97c60d735881ea3797df151f5c3aa79f9409..57ae0399a5f4b9f950f85d42c98deb8bc5ae01d0 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 b12e47c576daeb8a3b3556f405e5c5fbea0d4007..8546eba8058e2285e7d7d7d98c65a72dd853376f 100644 (file)
@@ -86,7 +86,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 1aa09e87ca1dc3fc304ff01f8925525a9c72dcf5..7dab3028de41e6a41c27f23ebc27505c60fcc02e 100644 (file)
@@ -98,7 +98,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 6df812234c3f9cf4591a83375a2dc39b4745662f..ddc5534352526120f859da9a6fa71765a1bca029 100644 (file)
@@ -78,7 +78,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 f0e58e11db36ec56ed0bf2de730f198a64cec0a3..5431dea4fc178c0c4dde9bb9f32c0cf066dddff6 100644 (file)
@@ -60,7 +60,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 ca443b699fdcf3b2cf6b3866fadad8a008ff5c57..e41afdb066a3bb34bebddf77d532ded6c5745304 100644 (file)
@@ -131,7 +131,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 1276c077d701f2c8bf894d780a79d8938d05268e..47e17182527fd9713aa48c63c17e8e7c2bc7e4b0 100644 (file)
@@ -59,7 +59,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 0df25979939357b4b464e0aaa44586903ef7be44..315a6e48354f46ca04e3ba66d06295a4f8871ac9 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 2207e9ab98f702fb0a2b43ddf6d2a6d19a1d668d..991b7ba60e252f5a2e514d986de38fa25b1f5080 100644 (file)
@@ -56,7 +56,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 dc3d89fae741c7ff808cf38bc253e2287ef91d4c..01d6c855979a340db5ff90a436c9e4d19d16355e 100644 (file)
@@ -55,7 +55,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 a3da46d1362fbcd615ff4378cbc229b302d6e644..8b1b26b03b5b34aec2810c0acac8921df36b2e72 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 886add8b543b3c927db0e362f535a1c90ec6d1cb..ca080b422517f0476158eb2ba91907540f969316 100644 (file)
@@ -52,7 +52,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 8a33894892f1f1e0ca8b1b9612b470984aee89a9..6c5386bcf1270b059ad5d8851890a8d9531ef42c 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 02f5b4ba3b71c386ee2beb37db3e8c332e3aff34..c1cd95df1493d12cbffe1409c1d16ea994c8dc31 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 3a763b2a6a7730035488b5323ff32023c129f50e..2422b0282e99de34991f022d043dfc63bc90d478 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 8a0496e80722e349bfc59ffe405be2419b99cc02..6d2c31631d0e7c28952ce82eba199479c9313fe9 100644 (file)
@@ -114,7 +114,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 4d50b4f6b136f23d5c615acadf1ad28748e130cc..cba35cfee3e73db7f5b3c749961c08d1fa96bdfc 100644 (file)
@@ -8,6 +8,7 @@ package runtime
 
 import (
        "internal/abi"
+       "runtime/internal/atomic"
        "unsafe"
 )
 
@@ -250,7 +251,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 f8ae8e7c3014b7eb8a0ff28a5d517354ded709fc..bc392e4c54d436bdb01f317dfdc51ac95b30d363 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