runtime,x/sys/windows: panic accessing (possibly) nil field of net_op #58870
Description
First of all, thank you all for everything that you do!
What version of Go are you using (go version)?
$ go version go version go1.20.1 windows/amd64
Does this issue reproduce with the latest release?
No, but reproducing the issue is timing dependent so my inability to reproduce seems to be the result of a different order of scheduling timeouts, etc.
What operating system and processor architecture are you using (go env)?
On Windows on amd64 hardware.
What did you do?
Using windows.WSAIoctl on an overlapped socket with a "zero" values in the windows.Overlapped struct will cause a panic in netpoll_windows.go at line 134 (as of the version of the code tagged at version 1.20.1) where the runtime attempts to dereference op.pd without checking whether it is nil.
The problem is that the runtime does not expect that something other than itself (i.e., the runtime) will add items to the list of operations that could be returned by a call to GetQueuedCompletionStatusEx. The comment
op *net_op // In reality it's *overlapped, but we cast it to *net_op anyway.is telling.
Unfortunately, we cannot guarantee that is the case.
If an application added something to that list (e.g., through a use of windows.WSAIoctl) that does not complete before the scheduler does another "round" of netpoll, it is possible that op is not actually a net_op. If the application user does "the right thing" by passing a pointer to a "zerod" windows.Overlapped structure, the runtime will break.
There was a related issue from several years earlier that @bradfitz found and fixed: #21172
This issue seems to be related.
I have two one-line patches to the sys and runtime packages that (I believe) provide a simple fix for the issue. I wanted to write this issue before posting them to gerrit so that there is something to reference. I will update this report with those patchsets.
See
sys/windows: Pad windows.Overlapped to protect the runtime | https://go-review.googlesource.com/c/sys/+/473163
and
473163: sys/windows: Pad windows.Overlapped to protect the runtime | https://go-review.googlesource.com/c/sys/+/473163