From: Quim Muntal Date: Mon, 5 Feb 2024 14:15:26 +0000 (+0000) Subject: Revert "runtime: make netpollBreak entries identifiable on Windows" X-Git-Tag: go1.23rc1~1290 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6abeffb18ea263b14cbe5936e8bdbbf08546e4b8;p=gostls13.git Revert "runtime: make netpollBreak entries identifiable on Windows" This reverts commit 29746b4814bb76a3d5a36fc86967b358f11bbb93. Reason for revert: Windows builders are flaky since this CL. Needs investigation. See https://build.golang.org/log/70d5d039b57b505870c9cc4e61de320df06a6f3a. Change-Id: I8a5874bb057785497d03b9450819578de7faeb47 Reviewed-on: https://go-review.googlesource.com/c/go/+/561276 Reviewed-by: David Chase Reviewed-by: Alex Brainman Reviewed-by: Bryan Mills LUCI-TryBot-Result: Go LUCI --- diff --git a/src/internal/poll/fd_windows_test.go b/src/internal/poll/fd_windows_test.go index 117da1aa1d..1cee18dcba 100644 --- a/src/internal/poll/fd_windows_test.go +++ b/src/internal/poll/fd_windows_test.go @@ -133,14 +133,24 @@ func TestWSASocketConflict(t *testing.T) { var outbuf _TCP_INFO_v0 cbbr := uint32(0) - var ov syscall.Overlapped + 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 + // Create an event so that we can efficiently wait for completion // of a requested overlapped I/O operation. - ov.HEvent, _ = windows.CreateEvent(nil, 0, 0, nil) - if ov.HEvent == 0 { + ovs[0].HEvent, _ = windows.CreateEvent(nil, 0, 0, nil) + if ovs[0].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)), @@ -148,7 +158,7 @@ func TestWSASocketConflict(t *testing.T) { (*byte)(unsafe.Pointer(&outbuf)), uint32(unsafe.Sizeof(outbuf)), &cbbr, - &ov, + &ovs[0], 0, ); err != nil && !errors.Is(err, syscall.ERROR_IO_PENDING) { t.Fatalf("could not perform the WSAIoctl: %v", err) @@ -157,12 +167,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(ov.HEvent, syscall.INFINITE); res != 0 { + if res, err := syscall.WaitForSingleObject(ovs[0].HEvent, syscall.INFINITE); res != 0 { t.Fatalf("waiting for the completion of the overlapped IO failed: %v", err) } } - if err = syscall.CloseHandle(ov.HEvent); err != nil { + if err = syscall.CloseHandle(ovs[0].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 7f68077d09..59377bc588 100644 --- a/src/runtime/netpoll_windows.go +++ b/src/runtime/netpoll_windows.go @@ -13,49 +13,18 @@ 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 - _ overlapped + o overlapped // used by netpoll pd *pollDesc mode int32 } type overlappedEntry struct { - key uintptr + key *pollDesc op *net_op // In reality it's *overlapped, but we cast it to *net_op anyway. internal uintptr qty uint32 @@ -80,19 +49,8 @@ func netpollIsPollDescriptor(fd uintptr) bool { } func netpollopen(fd uintptr, pd *pollDesc) int32 { - // 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 { + // TODO(iant): Consider using taggedPointer on 64-bit systems. + if stdcall4(_CreateIoCompletionPort, fd, iocphandle, uintptr(unsafe.Pointer(pd)), 0) == 0 { return int32(getlasterror()) } return 0 @@ -113,8 +71,7 @@ func netpollBreak() { return } - key := packNetpollKey(netpollSourceBreak, 0) - if stdcall4(_PostQueuedCompletionStatus, iocphandle, 0, key, 0) == 0 { + if stdcall4(_PostQueuedCompletionStatus, iocphandle, 0, 0, 0) == 0 { println("runtime: netpoll: PostQueuedCompletionStatus failed (errno=", getlasterror(), ")") throw("runtime: netpoll: PostQueuedCompletionStatus failed") } @@ -129,6 +86,7 @@ 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 @@ -169,31 +127,21 @@ func netpoll(delay int64) (gList, int32) { mp.blocked = false delta := int32(0) for i = 0; i < n; i++ { - 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. - 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 + 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, e.op.pd, mode) - default: - println("runtime: GetQueuedCompletionStatusEx returned net_op with invalid key=", e.key) - throw("runtime: netpoll failed") + delta += netpollready(&toRun, op.pd, mode) + } else { + netpollWakeSig.Store(0) + if delay == 0 { + // Forward the notification to the + // blocked poller. + netpollBreak() + } } } return toRun, delta