]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: make netpollBreak entries identifiable on Windows
authorqmuntal <quimmuntal@gmail.com>
Fri, 26 Jan 2024 16:57:39 +0000 (17:57 +0100)
committerQuim Muntal <quimmuntal@gmail.com>
Thu, 1 Feb 2024 19:29:37 +0000 (19:29 +0000)
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
src/runtime/netpoll_windows.go

index 1cee18dcba53fc8d0a4081f9e1071ac05226a24c..117da1aa1d38d3bd4c0cc0dea03566e0b8e3dfa3 100644 (file)
@@ -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)
        }
 }
index 59377bc5885d5678acc4f6e9eec4b8fb051e5529..7f68077d099bf221f29867f6a8dfe935bc5ae056 100644 (file)
@@ -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.
-       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