runtime: filter i/o async entries using completion key on windows

In the case where a user program requests overlapped I/O directly on a handlethat is managed by the runtime, it is possible that runtime.netpoll will attempt to dereference a pointer with an invalid value. This CL prevents the runtime from accessing the invalid pointer value by adding a special key to each overlapped I/O operation that it creates. Fixes #58870 Co-authored-by: quimmuntal@gmail.com Change-Id: Ib58ee757bb5555efba24c29101fc6d1a0dedd61a Reviewed-on: https://go-review.googlesource.com/c/go/+/482495 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Run-TryBot: Quim Muntal <quimmuntal@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org>
golang · Apr 11, 2023 · 7830180 · 7830180
1 parent 05cd6cb
commit 7830180
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 3 deletions.
diff --git a/src/internal/poll/fd_windows_test.go b/src/internal/poll/fd_windows_test.go
@@ -5,12 +5,15 @@
 package poll_test
 
 import (
+	"errors"
 	"fmt"
 	"internal/poll"
+	"internal/syscall/windows"
 	"os"
 	"sync"
 	"syscall"
 	"testing"
+	"unsafe"
 )
 
 type loggedFD struct {
@@ -109,3 +112,87 @@ func TestSerialFdsAreInitialised(t *testing.T) {
 		})
 	}
 }
+
+func TestWSASocketConflict(t *testing.T) {
+	s, err := windows.WSASocket(syscall.AF_INET, syscall.SOCK_STREAM, syscall.IPPROTO_TCP, nil, 0, windows.WSA_FLAG_OVERLAPPED)
+	if err != nil {
+		t.Fatal(err)
+	}
+	fd := poll.FD{Sysfd: s, IsStream: true, ZeroReadIsEOF: true}
+	_, err = fd.Init("tcp", true)
+	if err != nil {
+		syscall.CloseHandle(s)
+		t.Fatal(err)
+	}
+	defer fd.Close()
+
+	const SIO_TCP_INFO = syscall.IOC_INOUT | syscall.IOC_VENDOR | 39
+	inbuf := uint32(0)
+	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
+
+	// 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 {
+		t.Fatalf("could not create the event!")
+	}
+
+	// Set the low bit of the Event Handle so that the 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)),
+		uint32(unsafe.Sizeof(inbuf)),
+		(*byte)(unsafe.Pointer(&outbuf)),
+		uint32(unsafe.Sizeof(outbuf)),
+		&cbbr,
+		&ovs[0],
+		0,
+	); err != nil && !errors.Is(err, syscall.ERROR_IO_PENDING) {
+		t.Fatalf("could not perform the WSAIoctl: %v", err)
+	}
+
+	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 {
+			t.Fatalf("waiting for the completion of the overlapped IO failed: %v", err)
+		}
+	}
+
+	if err = syscall.CloseHandle(ovs[0].HEvent); err != nil {
+		t.Fatalf("could not close the event handle: %v", err)
+	}
+}
+
+type _TCP_INFO_v0 struct {
+	State             uint32
+	Mss               uint32
+	ConnectionTimeMs  uint64
+	TimestampsEnabled bool
+	RttUs             uint32
+	MinRttUs          uint32
+	BytesInFlight     uint32
+	Cwnd              uint32
+	SndWnd            uint32
+	RcvWnd            uint32
+	RcvBuf            uint32
+	BytesOut          uint64
+	BytesIn           uint64
+	BytesReordered    uint32
+	BytesRetrans      uint32
+	FastRetrans       uint32
+	DupAcksIn         uint32
+	TimeoutEpisodes   uint32
+	SynRetrans        uint8
+}
diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go
@@ -127,6 +127,12 @@ type IpAdapterAddresses struct {
 	/* more fields might be present here. */
 }
 
+type SecurityAttributes struct {
+	Length             uint16
+	SecurityDescriptor uintptr
+	InheritHandle      bool
+}
+
 type FILE_BASIC_INFO struct {
 	CreationTime   syscall.Filetime
 	LastAccessTime syscall.Filetime
@@ -370,6 +376,7 @@ func ErrorLoadingGetTempPath2() error {
 
 //sys	CreateEnvironmentBlock(block **uint16, token syscall.Token, inheritExisting bool) (err error) = userenv.CreateEnvironmentBlock
 //sys	DestroyEnvironmentBlock(block *uint16) (err error) = userenv.DestroyEnvironmentBlock
+//sys	CreateEvent(eventAttrs *SecurityAttributes, manualReset uint32, initialState uint32, name *uint16) (handle syscall.Handle, err error) = kernel32.CreateEventW
 
 //sys	RtlGenRandom(buf []byte) (err error) = advapi32.SystemFunction036
 

diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go
diff --git a/src/runtime/netpoll_windows.go b/src/runtime/netpoll_windows.go
@@ -26,7 +26,7 @@ type net_op struct {
 }
 
 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
@@ -51,7 +51,7 @@ func netpollIsPollDescriptor(fd uintptr) bool {
 }
 
 func netpollopen(fd uintptr, pd *pollDesc) int32 {
-	if stdcall4(_CreateIoCompletionPort, fd, iocphandle, 0, 0) == 0 {
+	if stdcall4(_CreateIoCompletionPort, fd, iocphandle, uintptr(unsafe.Pointer(pd)), 0) == 0 {
 		return int32(getlasterror())
 	}
 	return 0
@@ -128,7 +128,7 @@ func netpoll(delay int64) gList {
 	mp.blocked = false
 	for i = 0; i < n; i++ {
 		op = entries[i].op
-		if op != nil {
+		if op != nil && op.pd == entries[i].key {
 			errno = 0
 			qty = 0
 			if stdcall5(_WSAGetOverlappedResult, op.pd.fd, uintptr(unsafe.Pointer(op)), uintptr(unsafe.Pointer(&qty)), 0, uintptr(unsafe.Pointer(&flags))) == 0 {