UdpSocket receive to short buffer behaves differently on Unix and Windows

[andrewtj]

On Unix receiving to a buffer shorter than the incoming payload silently truncates the read. On Windows the read is completed but an error is returned. Not being that familiar with Windows I found this behaviour surprising.

The following comment in sys/windows/net.rs suggests that making Unix and Windows behave the same is (at times) a goal of the standard library:

        // On unix when a socket is shut down all further reads return 0, so we
        // do the same on windows to map a shut down socket to returning EOF.

If it is I'd like to propose that WSAEMSGSIZE be absorbed so that both platforms behave the same.

[andrewtj]

mio emulates the standard library and so tokio inherits this behaviour too. So if I'm not an outlier in finding this behaviour unusual it's likely that this error is mishandled in a lot of code.

To test that supposition I thought I'd do a quick survey of servers. I didn't find many that I could easily run but here are the results:

• @bluejekyll's named (trust-dns): panics
• @DarinM223's tftp-server: logs the error and appears to stop processing packets
• @djc's quinn (quic): can't fail as it receives into a 65,536 byte buffer
• @krolaw's dhcp4r: receives into a 1,500 byte buffer and stops server loop on all errors

[djc]

Cc @Ralith as well.

[Ralith]

Is the current Unix behavior the right default? If quinn were to ever receive into a shorter buffer (which is possible, since maximum packet size is negotiated by QUIC and memory isn't always cheap), we might prefer to detect and discard truncated packets, since they would be deemed malformed soon after anyway. Note that the windows behavior should be implementable on POSIX-compatible systems by using recvmsg and checking for MSG_TRUNC.

Note that returning an error doesn't render read data unusable, because by definition, if the message was truncated the entire supplied buffer was filled.

[bluejekyll]

@bluejekyll's named (trust-dns): panics

Do you happen to have a backtrace for this?

[andrewtj]

Is the current Unix behavior the right default?

I hadn't really thought about that. Are there any other errors that could require special handling other than perhaps EINTR? (Which is a consideration for blocking sockets only, right?)

[andrewtj]

@bluejekyll https://gist.github.com/andrewtj/8429d9f3b9396e0a4170c8e95d92b7a7

[Ralith]

Are there any other errors that could require special handling

The other weird Linux UDP behaviors I'm aware of are:

• sendmsg rarely yields EPERM, I think when the kernel's buffers are full and the datagram was therefore dropped locally
• recvmsg yields ECONNRESET when an ICMP destination unreachable(?) message is received, which is sometimes useful, but certainly shouldn't kill a server, and e.g. is ignored outright by quinn because an off-path attacker can spoof them with impunity.

[andrewtj]

Closing due to inactivity.

[bluejekyll]

Isn’t this worth keeping open? It seems like a pretty wide open exploit potential on Windows.

[Ralith]

It seems like a decision still needs to be made for which behavior to favor, if any. I made an argument for the Windows default to be adopted above.

[retep998]

3 months seems hardly like enough time to close for inactivity, and there's definitely some unresolved concerns here.

I'm definitely in favor of returning an error when the input buffer was too small for the incoming packet. Silently truncating is not the right approach to making a robust standard library.

[bluejekyll]

It seems like a decision still needs to be made for which behavior to favor, if any. I made an argument for the Windows default to be adopted above.

I didn't mean to imply what the solution should be, only that currently, it looks like many of us have some issues to resolve, that the stdlib should most likely help compensate for.

[Ralith]

Right, I'm just agreeing that there's an unsolved problem here.

[andrewtj]

I just noticed that the example for UdpSocket suggests an error won't be returned:

        // Receives a single datagram message on the socket. If `buf` is too small to hold
        // the message, it will be cut off.

(I'm only mentioning this as it may need to be amended.)

[oxalica]

What's the status of the issue?

I prefer emitting an error in this case like the default behavior on windows.
It makes users possible to choose to handle or ignore it.
Otherwise this information is simply lost when Ok(buffer_len) is returned, and there is no way to check whether the message is truncated.
(Well, this can "break" some codes on unix, though I think truncated packet will eventually cause errors after the recv.)

Maybe we can add a new variant to ErrorKind ?

[Ralith]

What's the status of the issue?

Nobody's cared enough to work on it, I think.

[andrewtj]

Nobody's cared enough to work on it, I think.

I thought this was waiting on a decision from the libs team. What is the process here?

[retep998]

The correct process is to poke @rust-lang/libs and ask them to make a decision on this.

[frehberg]

AFAICS, a-priori one does not know if hte size of the incoming datagram will exceed the user's message buffer. So, on Unix it may be truncated, and on Windows we may get an error. AFAICS, the appication needs to deal with both cases.
AFAICS, the documentation should be extended, explaining the different use cases on Unix/Posix and Windows.

[retep998]

On Windows it is truncated and you get an error telling you it was truncated. On Unix it is truncated without any indication that it was truncated.