The code assumes the memory layout of SocketAddrV{4,6}

[faern]

This library casts std::net::SocketAddrV4 (and V6) into libc::sockaddr: https://github.com/rust-lang/socket2-rs/blob/b0f77842b3357dd571bb86b30b354bc55215f693/src/sockaddr.rs#L95-L115

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.

This internals forum thread is where this discussion started: https://internals.rust-lang.org/t/why-are-socketaddrv4-socketaddrv6-based-on-low-level-sockaddr-in-6/13321

mio does the same kind of invalid casting: tokio-rs/mio#1386

[Thomasdezeeuw]

@Nemo157 made me aware of this in tokio-rs/mio#1386. I didn't know about rust-lang/rust#78802, but if that will proceed than I'll fix both Mio and socket2 in the same way.

[faern]

I think this needs to be fixed no matter what happens with rust-lang/rust#78802. Because both these crates still rely on a non specified memory layout. Having these assumptions in commonly used crates makes it more troublesome for the standard library to evolve and change internal details that should not matter to the outside world.

[sfackler]

For reference, this assumption was made because Alex wrote the implementation of std's SocketAddr and then also wrote the implementation of this library.

[faern]

Fixed in #120

[faern]

@Thomasdezeeuw Thanks for the fast and smooth collaboration on getting this fixed in all the crates you maintain! I know you said you wanted to get something more merged before releasing mio, but is there anything blocking releasing a patch release of socket2 0.3.x with this fix?

[de-vri-es]

I think #117 needs to be merged before releasing, or Socket::new() would change to not setting the CLOEXEC flag. But that has some requests for changes and it needs to be rebased on #124 first, which itself isn't merged yet.

[Thomasdezeeuw]

@de-vri-es #117 and #124 are for the master branch which will become v0.4.x. @faern I'll prepare a release.