qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-17 06:04:47 -07:00
Code Releases Activity

runtime: ensure thread handle is valid in profileloop1

Browse Source 
On Windows, there is currently a race between unminit closing the
thread's handle and profileloop1 suspending the thread using its
handle. If another handle reuses the same handle value, this can lead
to unpredictable results.

To fix this, we protect the thread handle with a lock and duplicate it
under this lock in profileloop1 before using it.

This is going to become a much bigger problem with non-cooperative
preemption (#10958, #24543), which uses the same basic mechanism as
profileloop1.

Change-Id: I9d62b83051df8c03f3363344438e37781a69ce16
Reviewed-on: https://go-review.googlesource.com/c/go/+/207779
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
This commit is contained in:
Austin Clements 2019-11-18 20:21:38 -05:00
parent 91bb1d734e
commit 6fd467ee29
1 changed files with 25 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/runtime/os_windows.go
View File

@ -143,7 +143,8 @@ func tstart_stdcall(newm *m)
func ctrlhandler()
type mOS struct {
thread uintptr // thread handle; accessed atomically
threadLock mutex // protects "thread" and prevents closing
thread uintptr // thread handle
waitsema uintptr // semaphore for parking on locks
resumesema uintptr // semaphore to indicate suspend/resume
@ -814,7 +815,11 @@ func sigblock() {
func minit() {
var thandle uintptr
stdcall7(_DuplicateHandle, currentProcess, currentThread, currentProcess, uintptr(unsafe.Pointer(&thandle)), 0, 0, _DUPLICATE_SAME_ACCESS)
atomic.Storeuintptr(&getg().m.thread, thandle)
mp := getg().m
lock(&mp.threadLock)
mp.thread = thandle
unlock(&mp.threadLock)
// Query the true stack base from the OS. Currently we're
// running on a small assumed stack.
@ -847,9 +852,11 @@ func minit() {
// Called from dropm to undo the effect of an minit.
//go:nosplit
func unminit() {
tp := &getg().m.thread
stdcall1(_CloseHandle, *tp)
*tp = 0
mp := getg().m
lock(&mp.threadLock)
stdcall1(_CloseHandle, mp.thread)
mp.thread = 0
unlock(&mp.threadLock)
}
// Calling stdcall on os stack.
@ -1018,17 +1025,25 @@ func profileloop1(param uintptr) uint32 {
stdcall2(_WaitForSingleObject, profiletimer, _INFINITE)
first := (*m)(atomic.Loadp(unsafe.Pointer(&allm)))
for mp := first; mp != nil; mp = mp.alllink {
thread := atomic.Loaduintptr(&mp.thread)
lock(&mp.threadLock)
// Do not profile threads blocked on Notes,
// this includes idle worker threads,
// idle timer thread, idle heap scavenger, etc.
if thread == 0 || mp.profilehz == 0 || mp.blocked {
if mp.thread == 0 || mp.profilehz == 0 || mp.blocked {
unlock(&mp.threadLock)
continue
}
// mp may exit between the load above and the
// SuspendThread, so be careful.
// Acquire our own handle to the thread.
var thread uintptr
stdcall7(_DuplicateHandle, currentProcess, mp.thread, currentProcess, uintptr(unsafe.Pointer(&thread)), 0, 0, _DUPLICATE_SAME_ACCESS)
unlock(&mp.threadLock)
// mp may exit between the DuplicateHandle
// above and the SuspendThread. The handle
// will remain valid, but SuspendThread may
// fail.
if int32(stdcall1(_SuspendThread, thread)) == -1 {
// The thread no longer exists.
stdcall1(_CloseHandle, thread)
continue
}
if mp.profilehz != 0 && !mp.blocked {
@ -1037,6 +1052,7 @@ func profileloop1(param uintptr) uint32 {
profilem(mp, thread)
}
stdcall1(_ResumeThread, thread)
stdcall1(_CloseHandle, thread)
}
}
}