diff options
author | Jason A. Donenfeld <Jason@zx2c4.com> | 2021-04-26 22:07:03 -0400 |
---|---|---|
committer | Jason A. Donenfeld <Jason@zx2c4.com> | 2021-04-26 22:07:03 -0400 |
commit | 8246d251ea9b9cbef07082bd69280a8f988cec0c (patch) | |
tree | ae8c85de45efcf73eca721b1af9169b1d3874cf5 | |
parent | c9db4b7aaa22b29f1c7c05ee8e3125ec2cab6965 (diff) |
conn: windows: do not error out when receiving UDP jumbogram
If we receive a large UDP packet, don't return an error to receive.go,
which then terminates the receive loop. Instead, simply retry.
Considering Winsock's general finickiness, we might consider other
places where an attacker on the wire can generate error conditions like
this.
Reported-by: Sascha Dierberg <sascha.dierberg@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
-rw-r--r-- | conn/bind_windows.go | 16 |
1 files changed, 14 insertions, 2 deletions
diff --git a/conn/bind_windows.go b/conn/bind_windows.go index fdd1c24..42208b1 100644 --- a/conn/bind_windows.go +++ b/conn/bind_windows.go @@ -352,8 +352,11 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e } bind.rx.mu.Lock() defer bind.rx.mu.Unlock() + + var err error var count uint32 var results [1]winrio.Result +retry: for tries := 0; count == 0 && tries < receiveSpins; tries++ { if tries > 0 { if atomic.LoadUint32(isOpen) != 1 { @@ -364,7 +367,7 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e count = winrio.DequeueCompletion(bind.rx.cq, results[:]) } if count == 0 { - err := winrio.Notify(bind.rx.cq) + err = winrio.Notify(bind.rx.cq) if err != nil { return 0, nil, err } @@ -385,10 +388,19 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e } } bind.rx.Return(1) - err := bind.InsertReceiveRequest() + err = bind.InsertReceiveRequest() if err != nil { return 0, nil, err } + // We limit the MTU well below the 65k max for practicality, but this means a remote host can still send us + // huge packets. Just try again when this happens. The infinite loop this could cause is still limited to + // attacker bandwidth, just like the rest of the receive path. + if windows.Errno(results[0].Status) == windows.WSAEMSGSIZE { + if atomic.LoadUint32(isOpen) != 1 { + return 0, nil, net.ErrClosed + } + goto retry + } if results[0].Status != 0 { return 0, nil, windows.Errno(results[0].Status) } |