Invalid transmute from `std::net::Ipv4Addr` to `libc::in_addr`

[stevenengler]

The functions ipv4addr_to_libc and ipv6addr_to_libc transmute from std::net::Ipv4Addr to libc::in_addr, but this is unsound.

nix/src/sys/socket/addr.rs

Lines 39 to 54 in 89b4976

	/// Convert a std::net::Ipv4Addr into the libc form.
	#[cfg(feature = "net")]
	pub(crate) const fn ipv4addr_to_libc(addr: net::Ipv4Addr) -> libc::in_addr {
	static_assertions::assert_eq_size!(net::Ipv4Addr, libc::in_addr);
	// Safe because both types have the same memory layout, and no fancy Drop
	// impls.
	unsafe { mem::transmute(addr) }
	}
	/// Convert a std::net::Ipv6Addr into the libc form.
	#[cfg(feature = "net")]
	pub(crate) const fn ipv6addr_to_libc(addr: &net::Ipv6Addr) -> libc::in6_addr {
	static_assertions::assert_eq_size!(net::Ipv6Addr, libc::in6_addr);
	// Safe because both are Newtype wrappers around the same libc type
	unsafe { mem::transmute(*addr) }
	}

The rust socket address types do not have the same layouts as their corresponding libc types, even if their size is the same.

See rust-lang/rust#78802 for details.

This release changes the memory layout of Ipv4Addr, Ipv6Addr, SocketAddrV4 and SocketAddrV6. The standard library no longer implements these as the corresponding libc structs (sockaddr_in, sockaddr_in6 etc.). This internal representation was never exposed, but some crates relied on it anyway by unsafely transmuting. This change will cause those crates to make invalid memory accesses.

This change obviously changes the memory layout of the types. And it turns out some libraries invalidly assumes the memory layout and does very dangerous pointer casts to convert them. These libraries will have undefined behaviour and perform invalid memory access until patched.

[WhyNotHugo]

Looks like these functions will have to be rewritten to perform a more expensive conversion rather than simply transmuting.

[asomers]

Annoying. I guess we'll have to change it. However, it looks like the standard library's new layout is still the same as the C layout, at least on common platforms. Do you know of any real platforms where it isn't?

[stevenengler]

Do you know of any real platforms where it isn't?

I don't know of any. Rust is free to do whatever it wants for the layout (and the layout isn't guaranteed to be the same even across builds), but it's probably unlikely that the rust representation of struct Ipv4Addr { octets: [u8; 4] } would ever be different from if the struct was repr(C). The Ipv6Addr and in6_addr have different alignments, but that shouldn't matter when transmuting the value rather than a reference.

But on the other hand, I would expect

libc::in_addr {
    s_addr: u32::from_be_bytes(addr.octets()).to_be()
}

to get optimized to essentially a no-op anyway without a need for transmute. For example on godbolt:

pub fn convert(addr: std::net::Ipv4Addr) -> u32 {
    u32::from_be_bytes(addr.octets()).to_be()
}

gets compiled to just

example::convert:
        mov     eax, edi
        ret