From f2b0149e5701939ebbe05449e58ab162d98d7a22 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 29 Nov 2021 15:07:04 -0500 Subject: [release-branch.go1.16] runtime: keep //go:cgo_unsafe_args arguments alive to prevent GC When syscall's DLL.FindProc calls into syscall_getprocaddress with a byte slice pointer, we need to keep those bytes alive. Otherwise the GC will collect the allocation, and we wind up calling `GetProcAddress` on garbage, which showed up as various flakes in the builders. It turns out that this problem extends to many uses of //go:cgo_unsafe_args throughout, on all platforms. So this patch fixes the issue by keeping non-integer pointer arguments alive through their invocation in //go:cgo_unsafe_args functions. Fixes #49867. Updates #49731. Change-Id: I93e4fbc2e8e210cb3fc53149708758bb33f2f9c7 Reviewed-on: https://go-review.googlesource.com/c/go/+/368356 Trust: Jason A. Donenfeld Run-TryBot: Jason A. Donenfeld TryBot-Result: Go Bot Reviewed-by: Michael Knyszek --- src/runtime/sys_darwin.go | 94 +++++++++++++++++++++++++++++++++-------- src/runtime/sys_darwin_arm64.go | 4 +- src/runtime/sys_openbsd.go | 22 +++++++--- src/runtime/sys_openbsd1.go | 5 ++- src/runtime/sys_openbsd2.go | 36 +++++++++++++--- src/runtime/syscall_solaris.go | 2 + src/runtime/syscall_windows.go | 4 ++ 7 files changed, 138 insertions(+), 29 deletions(-) diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go index 4a3f2fc453..bf124315df 100644 --- a/src/runtime/sys_darwin.go +++ b/src/runtime/sys_darwin.go @@ -102,28 +102,38 @@ func syscallNoErr() //go:nosplit //go:cgo_unsafe_args func pthread_attr_init(attr *pthreadattr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_init_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_init_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + return ret } func pthread_attr_init_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_attr_getstacksize(attr *pthreadattr, size *uintptr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_getstacksize_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_getstacksize_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + KeepAlive(size) + return ret } func pthread_attr_getstacksize_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_attr_setdetachstate(attr *pthreadattr, state int) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_setdetachstate_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_setdetachstate_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + return ret } func pthread_attr_setdetachstate_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_create(attr *pthreadattr, start uintptr, arg unsafe.Pointer) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_create_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_create_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + KeepAlive(arg) // Just for consistency. Arg of course needs to be kept alive for the start function. + return ret } func pthread_create_trampoline() @@ -172,6 +182,7 @@ func mmap_trampoline() //go:cgo_unsafe_args func munmap(addr unsafe.Pointer, n uintptr) { libcCall(unsafe.Pointer(funcPC(munmap_trampoline)), unsafe.Pointer(&addr)) + KeepAlive(addr) // Just for consistency. Hopefully addr is not a Go address. } func munmap_trampoline() @@ -179,6 +190,7 @@ func munmap_trampoline() //go:cgo_unsafe_args func madvise(addr unsafe.Pointer, n uintptr, flags int32) { libcCall(unsafe.Pointer(funcPC(madvise_trampoline)), unsafe.Pointer(&addr)) + KeepAlive(addr) // Just for consistency. Hopefully addr is not a Go address. } func madvise_trampoline() @@ -186,13 +198,16 @@ func madvise_trampoline() //go:cgo_unsafe_args func mlock(addr unsafe.Pointer, n uintptr) { libcCall(unsafe.Pointer(funcPC(mlock_trampoline)), unsafe.Pointer(&addr)) + KeepAlive(addr) // Just for consistency. Hopefully addr is not a Go address. } func mlock_trampoline() //go:nosplit //go:cgo_unsafe_args func read(fd int32, p unsafe.Pointer, n int32) int32 { - return libcCall(unsafe.Pointer(funcPC(read_trampoline)), unsafe.Pointer(&fd)) + ret := libcCall(unsafe.Pointer(funcPC(read_trampoline)), unsafe.Pointer(&fd)) + KeepAlive(p) + return ret } func read_trampoline() @@ -230,14 +245,18 @@ func usleep_trampoline() //go:nosplit //go:cgo_unsafe_args func write1(fd uintptr, p unsafe.Pointer, n int32) int32 { - return libcCall(unsafe.Pointer(funcPC(write_trampoline)), unsafe.Pointer(&fd)) + ret := libcCall(unsafe.Pointer(funcPC(write_trampoline)), unsafe.Pointer(&fd)) + KeepAlive(p) + return ret } func write_trampoline() //go:nosplit //go:cgo_unsafe_args func open(name *byte, mode, perm int32) (ret int32) { - return libcCall(unsafe.Pointer(funcPC(open_trampoline)), unsafe.Pointer(&name)) + ret = libcCall(unsafe.Pointer(funcPC(open_trampoline)), unsafe.Pointer(&name)) + KeepAlive(name) + return } func open_trampoline() @@ -276,6 +295,8 @@ func walltime_trampoline() //go:cgo_unsafe_args func sigaction(sig uint32, new *usigactiont, old *usigactiont) { libcCall(unsafe.Pointer(funcPC(sigaction_trampoline)), unsafe.Pointer(&sig)) + KeepAlive(new) + KeepAlive(old) } func sigaction_trampoline() @@ -283,6 +304,8 @@ func sigaction_trampoline() //go:cgo_unsafe_args func sigprocmask(how uint32, new *sigset, old *sigset) { libcCall(unsafe.Pointer(funcPC(sigprocmask_trampoline)), unsafe.Pointer(&how)) + KeepAlive(new) + KeepAlive(old) } func sigprocmask_trampoline() @@ -297,6 +320,8 @@ func sigaltstack(new *stackt, old *stackt) { new.ss_size = 32768 } libcCall(unsafe.Pointer(funcPC(sigaltstack_trampoline)), unsafe.Pointer(&new)) + KeepAlive(new) + KeepAlive(old) } func sigaltstack_trampoline() @@ -311,20 +336,32 @@ func raiseproc_trampoline() //go:cgo_unsafe_args func setitimer(mode int32, new, old *itimerval) { libcCall(unsafe.Pointer(funcPC(setitimer_trampoline)), unsafe.Pointer(&mode)) + KeepAlive(new) + KeepAlive(old) } func setitimer_trampoline() //go:nosplit //go:cgo_unsafe_args func sysctl(mib *uint32, miblen uint32, oldp *byte, oldlenp *uintptr, newp *byte, newlen uintptr) int32 { - return libcCall(unsafe.Pointer(funcPC(sysctl_trampoline)), unsafe.Pointer(&mib)) + ret := libcCall(unsafe.Pointer(funcPC(sysctl_trampoline)), unsafe.Pointer(&mib)) + KeepAlive(mib) + KeepAlive(oldp) + KeepAlive(oldlenp) + KeepAlive(newp) + return ret } func sysctl_trampoline() //go:nosplit //go:cgo_unsafe_args func sysctlbyname(name *byte, oldp *byte, oldlenp *uintptr, newp *byte, newlen uintptr) int32 { - return libcCall(unsafe.Pointer(funcPC(sysctlbyname_trampoline)), unsafe.Pointer(&name)) + ret := libcCall(unsafe.Pointer(funcPC(sysctlbyname_trampoline)), unsafe.Pointer(&name)) + KeepAlive(name) + KeepAlive(oldp) + KeepAlive(oldlenp) + KeepAlive(newp) + return ret } func sysctlbyname_trampoline() @@ -346,56 +383,79 @@ func kqueue_trampoline() //go:nosplit //go:cgo_unsafe_args func kevent(kq int32, ch *keventt, nch int32, ev *keventt, nev int32, ts *timespec) int32 { - return libcCall(unsafe.Pointer(funcPC(kevent_trampoline)), unsafe.Pointer(&kq)) + ret := libcCall(unsafe.Pointer(funcPC(kevent_trampoline)), unsafe.Pointer(&kq)) + KeepAlive(ch) + KeepAlive(ev) + KeepAlive(ts) + return ret } func kevent_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_mutex_init(m *pthreadmutex, attr *pthreadmutexattr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_mutex_init_trampoline)), unsafe.Pointer(&m)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_mutex_init_trampoline)), unsafe.Pointer(&m)) + KeepAlive(m) + KeepAlive(attr) + return ret } func pthread_mutex_init_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_mutex_lock(m *pthreadmutex) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_mutex_lock_trampoline)), unsafe.Pointer(&m)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_mutex_lock_trampoline)), unsafe.Pointer(&m)) + KeepAlive(m) + return ret } func pthread_mutex_lock_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_mutex_unlock(m *pthreadmutex) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_mutex_unlock_trampoline)), unsafe.Pointer(&m)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_mutex_unlock_trampoline)), unsafe.Pointer(&m)) + KeepAlive(m) + return ret } func pthread_mutex_unlock_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_cond_init(c *pthreadcond, attr *pthreadcondattr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_cond_init_trampoline)), unsafe.Pointer(&c)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_cond_init_trampoline)), unsafe.Pointer(&c)) + KeepAlive(c) + KeepAlive(attr) + return ret } func pthread_cond_init_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_cond_wait(c *pthreadcond, m *pthreadmutex) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_cond_wait_trampoline)), unsafe.Pointer(&c)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_cond_wait_trampoline)), unsafe.Pointer(&c)) + KeepAlive(c) + KeepAlive(m) + return ret } func pthread_cond_wait_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_cond_timedwait_relative_np(c *pthreadcond, m *pthreadmutex, t *timespec) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_cond_timedwait_relative_np_trampoline)), unsafe.Pointer(&c)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_cond_timedwait_relative_np_trampoline)), unsafe.Pointer(&c)) + KeepAlive(c) + KeepAlive(m) + KeepAlive(t) + return ret } func pthread_cond_timedwait_relative_np_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_cond_signal(c *pthreadcond) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_cond_signal_trampoline)), unsafe.Pointer(&c)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_cond_signal_trampoline)), unsafe.Pointer(&c)) + KeepAlive(c) + return ret } func pthread_cond_signal_trampoline() diff --git a/src/runtime/sys_darwin_arm64.go b/src/runtime/sys_darwin_arm64.go index 9c14f33a1c..471e93cb71 100644 --- a/src/runtime/sys_darwin_arm64.go +++ b/src/runtime/sys_darwin_arm64.go @@ -14,7 +14,9 @@ import ( //go:nosplit //go:cgo_unsafe_args func g0_pthread_key_create(k *pthreadkey, destructor uintptr) int32 { - return asmcgocall(unsafe.Pointer(funcPC(pthread_key_create_trampoline)), unsafe.Pointer(&k)) + ret := asmcgocall(unsafe.Pointer(funcPC(pthread_key_create_trampoline)), unsafe.Pointer(&k)) + KeepAlive(k) + return ret } func pthread_key_create_trampoline() diff --git a/src/runtime/sys_openbsd.go b/src/runtime/sys_openbsd.go index fcddf4d6a5..bda8a32ae8 100644 --- a/src/runtime/sys_openbsd.go +++ b/src/runtime/sys_openbsd.go @@ -14,35 +14,47 @@ import "unsafe" //go:nosplit //go:cgo_unsafe_args func pthread_attr_init(attr *pthreadattr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_init_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_init_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + return ret } func pthread_attr_init_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_attr_destroy(attr *pthreadattr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_destroy_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_destroy_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + return ret } func pthread_attr_destroy_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_attr_getstacksize(attr *pthreadattr, size *uintptr) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_getstacksize_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_getstacksize_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + KeepAlive(size) + return ret } func pthread_attr_getstacksize_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_attr_setdetachstate(attr *pthreadattr, state int) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_attr_setdetachstate_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_attr_setdetachstate_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + return ret } func pthread_attr_setdetachstate_trampoline() //go:nosplit //go:cgo_unsafe_args func pthread_create(attr *pthreadattr, start uintptr, arg unsafe.Pointer) int32 { - return libcCall(unsafe.Pointer(funcPC(pthread_create_trampoline)), unsafe.Pointer(&attr)) + ret := libcCall(unsafe.Pointer(funcPC(pthread_create_trampoline)), unsafe.Pointer(&attr)) + KeepAlive(attr) + KeepAlive(arg) // Just for consistency. Arg of course needs to be kept alive for the start function. + return ret } func pthread_create_trampoline() diff --git a/src/runtime/sys_openbsd1.go b/src/runtime/sys_openbsd1.go index e2886218db..08a978b4ae 100644 --- a/src/runtime/sys_openbsd1.go +++ b/src/runtime/sys_openbsd1.go @@ -11,7 +11,10 @@ import "unsafe" //go:nosplit //go:cgo_unsafe_args func thrsleep(ident uintptr, clock_id int32, tsp *timespec, lock uintptr, abort *uint32) int32 { - return libcCall(unsafe.Pointer(funcPC(thrsleep_trampoline)), unsafe.Pointer(&ident)) + ret := libcCall(unsafe.Pointer(funcPC(thrsleep_trampoline)), unsafe.Pointer(&ident)) + KeepAlive(tsp) + KeepAlive(abort) + return ret } func thrsleep_trampoline() diff --git a/src/runtime/sys_openbsd2.go b/src/runtime/sys_openbsd2.go index 474e7145e7..5303a68cf5 100644 --- a/src/runtime/sys_openbsd2.go +++ b/src/runtime/sys_openbsd2.go @@ -53,6 +53,7 @@ func mmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uint32) (un ret2 int }{addr, n, prot, flags, fd, off, nil, 0} libcCall(unsafe.Pointer(funcPC(mmap_trampoline)), unsafe.Pointer(&args)) + KeepAlive(addr) // Just for consistency. Hopefully addr is not a Go address. return args.ret1, args.ret2 } func mmap_trampoline() @@ -61,6 +62,7 @@ func mmap_trampoline() //go:cgo_unsafe_args func munmap(addr unsafe.Pointer, n uintptr) { libcCall(unsafe.Pointer(funcPC(munmap_trampoline)), unsafe.Pointer(&addr)) + KeepAlive(addr) // Just for consistency. Hopefully addr is not a Go address. } func munmap_trampoline() @@ -68,13 +70,16 @@ func munmap_trampoline() //go:cgo_unsafe_args func madvise(addr unsafe.Pointer, n uintptr, flags int32) { libcCall(unsafe.Pointer(funcPC(madvise_trampoline)), unsafe.Pointer(&addr)) + KeepAlive(addr) // Just for consistency. Hopefully addr is not a Go address. } func madvise_trampoline() //go:nosplit //go:cgo_unsafe_args func open(name *byte, mode, perm int32) (ret int32) { - return libcCall(unsafe.Pointer(funcPC(open_trampoline)), unsafe.Pointer(&name)) + ret = libcCall(unsafe.Pointer(funcPC(open_trampoline)), unsafe.Pointer(&name)) + KeepAlive(name) + return } func open_trampoline() @@ -88,14 +93,18 @@ func close_trampoline() //go:nosplit //go:cgo_unsafe_args func read(fd int32, p unsafe.Pointer, n int32) int32 { - return libcCall(unsafe.Pointer(funcPC(read_trampoline)), unsafe.Pointer(&fd)) + ret := libcCall(unsafe.Pointer(funcPC(read_trampoline)), unsafe.Pointer(&fd)) + KeepAlive(p) + return ret } func read_trampoline() //go:nosplit //go:cgo_unsafe_args func write1(fd uintptr, p unsafe.Pointer, n int32) int32 { - return libcCall(unsafe.Pointer(funcPC(write_trampoline)), unsafe.Pointer(&fd)) + ret := libcCall(unsafe.Pointer(funcPC(write_trampoline)), unsafe.Pointer(&fd)) + KeepAlive(p) + return ret } func write_trampoline() @@ -118,6 +127,8 @@ func pipe2_trampoline() //go:cgo_unsafe_args func setitimer(mode int32, new, old *itimerval) { libcCall(unsafe.Pointer(funcPC(setitimer_trampoline)), unsafe.Pointer(&mode)) + KeepAlive(new) + KeepAlive(old) } func setitimer_trampoline() @@ -131,7 +142,12 @@ func usleep_trampoline() //go:nosplit //go:cgo_unsafe_args func sysctl(mib *uint32, miblen uint32, out *byte, size *uintptr, dst *byte, ndst uintptr) int32 { - return libcCall(unsafe.Pointer(funcPC(sysctl_trampoline)), unsafe.Pointer(&mib)) + ret := libcCall(unsafe.Pointer(funcPC(sysctl_trampoline)), unsafe.Pointer(&mib)) + KeepAlive(mib) + KeepAlive(out) + KeepAlive(size) + KeepAlive(dst) + return ret } func sysctl_trampoline() @@ -175,7 +191,11 @@ func kqueue_trampoline() //go:nosplit //go:cgo_unsafe_args func kevent(kq int32, ch *keventt, nch int32, ev *keventt, nev int32, ts *timespec) int32 { - return libcCall(unsafe.Pointer(funcPC(kevent_trampoline)), unsafe.Pointer(&kq)) + ret := libcCall(unsafe.Pointer(funcPC(kevent_trampoline)), unsafe.Pointer(&kq)) + KeepAlive(ch) + KeepAlive(ev) + KeepAlive(ts) + return ret } func kevent_trampoline() @@ -183,6 +203,8 @@ func kevent_trampoline() //go:cgo_unsafe_args func sigaction(sig uint32, new *sigactiont, old *sigactiont) { libcCall(unsafe.Pointer(funcPC(sigaction_trampoline)), unsafe.Pointer(&sig)) + KeepAlive(new) + KeepAlive(old) } func sigaction_trampoline() @@ -190,6 +212,8 @@ func sigaction_trampoline() //go:cgo_unsafe_args func sigprocmask(how uint32, new *sigset, old *sigset) { libcCall(unsafe.Pointer(funcPC(sigprocmask_trampoline)), unsafe.Pointer(&how)) + KeepAlive(new) + KeepAlive(old) } func sigprocmask_trampoline() @@ -197,6 +221,8 @@ func sigprocmask_trampoline() //go:cgo_unsafe_args func sigaltstack(new *stackt, old *stackt) { libcCall(unsafe.Pointer(funcPC(sigaltstack_trampoline)), unsafe.Pointer(&new)) + KeepAlive(new) + KeepAlive(old) } func sigaltstack_trampoline() diff --git a/src/runtime/syscall_solaris.go b/src/runtime/syscall_solaris.go index 094516927f..ed5b7d19f9 100644 --- a/src/runtime/syscall_solaris.go +++ b/src/runtime/syscall_solaris.go @@ -303,6 +303,8 @@ func syscall_wait4(pid uintptr, wstatus *uint32, options uintptr, rusage unsafe. entersyscallblock() asmcgocall(unsafe.Pointer(&asmsysvicall6x), unsafe.Pointer(&call)) exitsyscall() + KeepAlive(wstatus) + KeepAlive(rusage) return int(call.r1), call.err } diff --git a/src/runtime/syscall_windows.go b/src/runtime/syscall_windows.go index 7835b492f7..296f6a39c1 100644 --- a/src/runtime/syscall_windows.go +++ b/src/runtime/syscall_windows.go @@ -276,6 +276,8 @@ func syscall_loadsystemlibrary(filename *uint16, absoluteFilepath *uint16) (hand } cgocall(asmstdcallAddr, unsafe.Pointer(c)) + KeepAlive(filename) + KeepAlive(absoluteFilepath) handle = c.r1 if handle == 0 { err = c.err @@ -294,6 +296,7 @@ func syscall_loadlibrary(filename *uint16) (handle, err uintptr) { c.n = 1 c.args = uintptr(noescape(unsafe.Pointer(&filename))) cgocall(asmstdcallAddr, unsafe.Pointer(c)) + KeepAlive(filename) handle = c.r1 if handle == 0 { err = c.err @@ -311,6 +314,7 @@ func syscall_getprocaddress(handle uintptr, procname *byte) (outhandle, err uint c.n = 2 c.args = uintptr(noescape(unsafe.Pointer(&handle))) cgocall(asmstdcallAddr, unsafe.Pointer(c)) + KeepAlive(procname) outhandle = c.r1 if outhandle == 0 { err = c.err -- cgit v1.2.3-54-g00ecf