mio::sys::unix::net::socket_addr assumes the layout of std::net::SocketAddrV{4,6} matches libc::sockaddr

[Nemo157]

@faern noted this in https://internals.rust-lang.org/t/why-are-socketaddrv4-socketaddrv6-based-on-low-level-sockaddr-in-6/13321

mio/src/sys/unix/net.rs

Lines 78 to 85 in 27fbd5f

	SocketAddr::V4(ref addr) => (
	addr as *const _ as *const libc::sockaddr,
	size_of_val(addr) as libc::socklen_t,
	),
	SocketAddr::V6(ref addr) => (
	addr as *const _ as *const libc::sockaddr,
	size_of_val(addr) as libc::socklen_t,
	),

As far as I can tell there are no guarantees from std about the layout of SocketAddrV{4,6}, and this code could silently compile and cause UB elsewhere if the representation changes (e.g. if padding is added early on in the struct, resulting in C code reading uninitialized bytes).

[Thomasdezeeuw]

This is true, but as far as I known there (currently) is no other way to do this. I'm open to suggestions.

I think socket2 has a similar problem: https://github.com/rust-lang/socket2-rs/blob/b0f77842b3357dd571bb86b30b354bc55215f693/src/sockaddr.rs#L95-L115

[Thomasdezeeuw]

Maybe we can add an assertion size_of_val(addr) == size_of(sockaddr_in), it doesn't solve the problem but at least it won't compile (and cause UB).

[Nemo157]

It'll help slightly, but doesn't guarantee no UB.

Fixing socket2 seems easier since it only ever copies values around, something like https://gist.github.com/Nemo157/00787cd835f73bbf90b12cbf71122844 should work (EDIT: I'm not sure about the endianness in that patch, tests pass, but maybe it's using the wrong port number 😁). Fixing mio would take more rearranging since it's not possible to go &SocketAddr -> *const sockaddr soundly.

[Thomasdezeeuw]

Related: rust-lang/socket2#119.