From: qmuntal Date: Fri, 26 Jan 2024 16:57:39 +0000 (+0100) Subject: runtime: make netpollBreak entries identifiable on Windows X-Git-Tag: go1.23rc1~1324 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=29746b4814bb76a3d5a36fc86967b358f11bbb93;p=gostls13.git runtime: make netpollBreak entries identifiable on Windows It is currently not possible to distinguish between a netpollBreak entry and an entry initiated by external WSA operations (as in #58870). This CL sets a unique completion key when posting the netpollBreak entry so that it can be identified as such. Change-Id: I8e74a7ddc607dc215d6ed8c59d5c3cf47ec8dc62 Reviewed-on: https://go-review.googlesource.com/c/go/+/558895 LUCI-TryBot-Result: Go LUCI Reviewed-by: Bryan Mills Reviewed-by: Michael Knyszek Reviewed-by: Alex Brainman --- diff --git a/src/internal/poll/fd_windows_test.go b/src/internal/poll/fd_windows_test.go index 1cee18dcba..117da1aa1d 100644 --- a/src/internal/poll/fd_windows_test.go +++ b/src/internal/poll/fd_windows_test.go @@ -133,24 +133,14 @@ func TestWSASocketConflict(t *testing.T) { var outbuf _TCP_INFO_v0 cbbr := uint32(0) - var ovs []syscall.Overlapped = make([]syscall.Overlapped, 2) - // Attempt to exercise behavior where a user-owned syscall.Overlapped - // induces an invalid pointer dereference in the Windows-specific version - // of runtime.netpoll. - ovs[1].Internal -= 1 - + var ov syscall.Overlapped // Create an event so that we can efficiently wait for completion // of a requested overlapped I/O operation. - ovs[0].HEvent, _ = windows.CreateEvent(nil, 0, 0, nil) - if ovs[0].HEvent == 0 { + ov.HEvent, _ = windows.CreateEvent(nil, 0, 0, nil) + if ov.HEvent == 0 { t.Fatalf("could not create the event!") } - // Set the low bit of the Event Handle so that the completion - // of the overlapped I/O event will not trigger a completion event - // on any I/O completion port associated with the handle. - ovs[0].HEvent |= 0x1 - if err = fd.WSAIoctl( SIO_TCP_INFO, (*byte)(unsafe.Pointer(&inbuf)), @@ -158,7 +148,7 @@ func TestWSASocketConflict(t *testing.T) { (*byte)(unsafe.Pointer(&outbuf)), uint32(unsafe.Sizeof(outbuf)), &cbbr, - &ovs[0], + &ov, 0, ); err != nil && !errors.Is(err, syscall.ERROR_IO_PENDING) { t.Fatalf("could not perform the WSAIoctl: %v", err) @@ -167,12 +157,12 @@ func TestWSASocketConflict(t *testing.T) { if err != nil && errors.Is(err, syscall.ERROR_IO_PENDING) { // It is possible that the overlapped I/O operation completed // immediately so there is no need to wait for it to complete. - if res, err := syscall.WaitForSingleObject(ovs[0].HEvent, syscall.INFINITE); res != 0 { + if res, err := syscall.WaitForSingleObject(ov.HEvent, syscall.INFINITE); res != 0 { t.Fatalf("waiting for the completion of the overlapped IO failed: %v", err) } } - if err = syscall.CloseHandle(ovs[0].HEvent); err != nil { + if err = syscall.CloseHandle(ov.HEvent); err != nil { t.Fatalf("could not close the event handle: %v", err) } } diff --git a/src/runtime/netpoll_windows.go b/src/runtime/netpoll_windows.go index 59377bc588..7f68077d09 100644 --- a/src/runtime/netpoll_windows.go +++ b/src/runtime/netpoll_windows.go @@ -13,18 +13,49 @@ const _DWORD_MAX = 0xffffffff const _INVALID_HANDLE_VALUE = ^uintptr(0) +// Sources are used to identify the event that created an overlapped entry. +// The source values are arbitrary. There is no risk of collision with user +// defined values because the only way to set the key of an overlapped entry +// is using the iocphandle, which is not accessible to user code. +const ( + netpollSourceReady = iota + 1 + netpollSourceBreak +) + +const ( + netPollKeySourceBits = 8 + // Use 19 bits for the fdseq, which is enough to represent all possible + // values on 64-bit systems (fdseq is truncated to taggedPointerBits). + // On 32-bit systems, taggedPointerBits is set to 32 bits, so we are + // losing precision here, but still have enough entropy to avoid collisions + // (see netpollopen). + netPollKeyFDSeqBits = 19 + netPollKeyFDSeqMask = 1<> netPollKeySourceBits +} + // net_op must be the same as beginning of internal/poll.operation. // Keep these in sync. type net_op struct { // used by windows - o overlapped + _ overlapped // used by netpoll pd *pollDesc mode int32 } type overlappedEntry struct { - key *pollDesc + key uintptr op *net_op // In reality it's *overlapped, but we cast it to *net_op anyway. internal uintptr qty uint32 @@ -49,8 +80,19 @@ func netpollIsPollDescriptor(fd uintptr) bool { } func netpollopen(fd uintptr, pd *pollDesc) int32 { - // TODO(iant): Consider using taggedPointer on 64-bit systems. - if stdcall4(_CreateIoCompletionPort, fd, iocphandle, uintptr(unsafe.Pointer(pd)), 0) == 0 { + // The tag is used for two purposes: + // - identify stale pollDescs. See go.dev/issue/59545. + // - differentiate between entries from internal/poll and entries from + // outside the Go runtime, which we want to skip. User code has access + // to fd, therefore it can run async operations on it that will end up + // adding overlapped entries to our iocp queue. See go.dev/issue/58870. + // By setting the tag to the pollDesc's fdseq, the only chance of + // collision is if a user creates an overlapped struct with a fdseq that + // matches the fdseq of the pollDesc passed to netpollopen, which is quite + // unlikely given that fdseq is not exposed to user code. + tag := pd.fdseq.Load() & netPollKeyFDSeqMask + key := packNetpollKey(netpollSourceReady, tag) + if stdcall4(_CreateIoCompletionPort, fd, iocphandle, key, 0) == 0 { return int32(getlasterror()) } return 0 @@ -71,7 +113,8 @@ func netpollBreak() { return } - if stdcall4(_PostQueuedCompletionStatus, iocphandle, 0, 0, 0) == 0 { + key := packNetpollKey(netpollSourceBreak, 0) + if stdcall4(_PostQueuedCompletionStatus, iocphandle, 0, key, 0) == 0 { println("runtime: netpoll: PostQueuedCompletionStatus failed (errno=", getlasterror(), ")") throw("runtime: netpoll: PostQueuedCompletionStatus failed") } @@ -86,7 +129,6 @@ func netpoll(delay int64) (gList, int32) { var entries [64]overlappedEntry var wait, n, i uint32 var errno int32 - var op *net_op var toRun gList mp := getg().m @@ -127,21 +169,31 @@ func netpoll(delay int64) (gList, int32) { mp.blocked = false delta := int32(0) for i = 0; i < n; i++ { - op = entries[i].op - if op != nil && op.pd == entries[i].key { - mode := op.mode - if mode != 'r' && mode != 'w' { - println("runtime: GetQueuedCompletionStatusEx returned net_op with invalid mode=", mode) - throw("runtime: netpoll failed") - } - delta += netpollready(&toRun, op.pd, mode) - } else { + e := &entries[i] + key, tag := unpackNetpollKey(e.key) + switch { + case key == netpollSourceBreak: netpollWakeSig.Store(0) if delay == 0 { - // Forward the notification to the - // blocked poller. + // Forward the notification to the blocked poller. netpollBreak() } + case key == netpollSourceReady: + if e.op == nil || e.op.pd == nil || e.op.pd.fdseq.Load()&netPollKeyFDSeqMask != tag&netPollKeyFDSeqMask { + // Stale entry or entry from outside the Go runtime and internal/poll, ignore. + // See go.dev/issue/58870. + continue + } + // Entry from internal/poll. + mode := e.op.mode + if mode != 'r' && mode != 'w' { + println("runtime: GetQueuedCompletionStatusEx returned net_op with invalid mode=", mode) + throw("runtime: netpoll failed") + } + delta += netpollready(&toRun, e.op.pd, mode) + default: + println("runtime: GetQueuedCompletionStatusEx returned net_op with invalid key=", e.key) + throw("runtime: netpoll failed") } } return toRun, delta