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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>

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)
+	var ov syscall.Overlapped
-	// 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.
-	ovs[0].HEvent, _ = windows.CreateEvent(nil, 0, 0, nil)
+	ov.HEvent, _ = windows.CreateEvent(nil, 0, 0, nil)
-	if ovs[0].HEvent == 0 {
+	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)
 	}
 }

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<<netPollKeyFDSeqBits - 1
+)
+
+// packNetpollKey creates a key from a source and a tag.
+// Tag bits that don't fit in the result are discarded.
+func packNetpollKey(source uint8, tag uintptr) uintptr {
+	return uintptr(source) | tag<<netPollKeySourceBits
+}
+
+// unpackNetpollKey returns the source and the tag from a taggedPointer.
+func unpackNetpollKey(key uintptr) (source uint8, tag uintptr) {
+	return uint8(key), key >> 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.
+	// The tag is used for two purposes:
-	if stdcall4(_CreateIoCompletionPort, fd, iocphandle, uintptr(unsafe.Pointer(pd)), 0) == 0 {
+	// - 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
+		e := &entries[i]
-		if op != nil && op.pd == entries[i].key {
+		key, tag := unpackNetpollKey(e.key)
-			mode := op.mode
+		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
 			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)
+			delta += netpollready(&toRun, e.op.pd, mode)
-		} else {
+		default:
-			netpollWakeSig.Store(0)
+			println("runtime: GetQueuedCompletionStatusEx returned net_op with invalid key=", e.key)
-			if delay == 0 {
+			throw("runtime: netpoll failed")
-				// Forward the notification to the
-				// blocked poller.
-				netpollBreak()
-			}
 		}
 	}
 	return toRun, delta