diff options
| author | bors <bors@rust-lang.org> | 2022-07-31 15:56:28 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-07-31 15:56:28 +0000 |
| commit | 3405e402faf413ef4633142225228c260a91dd9d (patch) | |
| tree | 1e9eba2e7f7cf89df5021ec2a6050f0357d5c3b4 /library/std/src/sys_common | |
| parent | e5a7d8f945f4259fb6b8c4953a65248243c054d1 (diff) | |
| parent | 73bb371ad3dd401a8f6dca518ad2bbf0bad590e5 (diff) | |
Auto merge of #78802 - faern:simplify-socketaddr, r=joshtriplett
Implement network primitives with ideal Rust layout, not C system layout
This PR is the result of this internals forum thread: https://internals.rust-lang.org/t/why-are-socketaddrv4-socketaddrv6-based-on-low-level-sockaddr-in-6/13321.
Instead of basing `std:::net::{Ipv4Addr, Ipv6Addr, SocketAddrV4, SocketAddrV6}` on system (C) structs, they are encoded in a more optimal and idiomatic Rust way.
This changes the public API of std by introducing structural equality impls for all four types here, which means that `match ipv4addr { SOME_CONSTANT => ... }` will now compile, whereas previously this was an error. No other intentional changes are introduced to public API.
It's possible to observe the current layout of these types (e.g., by pointer casting); most but not all libraries which were found by Crater to do this have had updates issued and affected versions yanked. See report below.
### Benefits of this change
- It will become possible to move these fundamental network types from `std` into `core` ([RFC](https://github.com/rust-lang/rfcs/pull/2832)).
- Some methods that can't be made `const fn`s today can be made `const fn`s with this change.
- `SocketAddrV4` only occupies 6 bytes instead of 16 bytes.
- These simple primitives become easier to read and uses less `unsafe`.
- Makes these types support structural equality, which means you can now (for instance) match an `Ipv4Addr` against a constant
### ~Remaining~ Previous problems
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.
- [x] - `mio` - Issue: https://github.com/tokio-rs/mio/issues/1386.
- [x] `0.7` branch https://github.com/tokio-rs/mio/pull/1388
- [x] `0.7.6` published https://github.com/tokio-rs/mio/pull/1398
- [x] Yank all `0.7` versions older than `0.7.6`
- [x] Report `<0.7.6` to RustSec Advisory Database https://rustsec.org/advisories/RUSTSEC-2020-0081.html
- [x] - `socket2` - Issue: https://github.com/rust-lang/socket2-rs/issues/119.
- [x] `0.3.x` branch https://github.com/rust-lang/socket2-rs/pull/120
- [x] `0.3.16` published
- [x] `master` branch https://github.com/rust-lang/socket2-rs/pull/122
- [x] Yank all `0.3` versions older than `0.3.16`
- [x] Report `<0.3.16` to RustSec Advisory Database https://rustsec.org/advisories/RUSTSEC-2020-0079.html
- [x] - `net2` - Issue: https://github.com/deprecrated/net2-rs/issues/105
- [x] https://github.com/deprecrated/net2-rs/pull/106
- [x] `0.2.36` published
- [x] Yank all `0.2` versions older than `0.2.36`
- [x] Report `<0.2.36` to RustSec Advisory Database https://rustsec.org/advisories/RUSTSEC-2020-0078.html
- [x] - `miow` - Issue: https://github.com/yoshuawuyts/miow/issues/38
- [x] `0.3.x` - https://github.com/yoshuawuyts/miow/pull/39
- [x] `0.3.6` published
- [x] `0.2.x` - https://github.com/yoshuawuyts/miow/pull/40
- [x] `0.2.2` published
- [x] Yanked all `0.2` versions older than `0.2.2`
- [x] Yanked all `0.3` versions older than `0.3.6`
- [x] Report `<0.2.2` and `<0.3.6` to RustSec Advisory Database https://rustsec.org/advisories/RUSTSEC-2020-0080.html
- [x] - `quinn master` (aka what became 0.7) - https://github.com/quinn-rs/quinn/issues/968 https://github.com/quinn-rs/quinn/pull/987
- [x] - `quinn 0.6` - https://github.com/quinn-rs/quinn/pull/1045
- [x] - `quinn 0.5` - https://github.com/quinn-rs/quinn/pull/1046
- [x] - Release `0.7.0`, `0.6.2` and `0.5.4`
- [x] - `nb-connect` - https://github.com/smol-rs/nb-connect/issues/1
- [x] - Release `1.0.3`
- [x] - Yank all versions older than `1.0.3`
- [x] - `shadowsocks-rust` - https://github.com/shadowsocks/shadowsocks-rust/issues/462
- [ ] - `rio` - https://github.com/spacejam/rio/issues/44
- [ ] - `seaslug` - https://github.com/spacejam/seaslug/issues/1
#### Fixed crate versions
All crates I have found that assumed the memory layout have been fixed and published. The crates and versions that will continue working even as/if this PR is merged is (please upgrade these to help unblock this PR):
* `net2 0.2.36`
* `socket2 0.3.16`
* `miow 0.2.2`
* `miow 0.3.6`
* `mio 0.7.6`
* `mio 0.6.23` - Never had the invalid assumption itself, but has now been bumped to only allow fixed dependencies (`net2` + `miow`)
* `nb-connect 1.0.3`
* `quinn 0.5.4`
* `quinn 0.6.2`
### Release notes draft
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. Notably `net2 <0.2.36`, `socket2 <0.3.16`, `mio <0.7.6`, `miow <0.3.6` and a few other crates are affected. All known affected crates have been patched and have had fixed versions published over a year ago. If any affected crate is still in your dependency tree, you need to upgrade them before using this version of Rust.
Diffstat (limited to 'library/std/src/sys_common')
| -rw-r--r-- | library/std/src/sys_common/net.rs | 61 |
1 files changed, 48 insertions, 13 deletions
diff --git a/library/std/src/sys_common/net.rs b/library/std/src/sys_common/net.rs index c13bda32823..33d336c4317 100644 --- a/library/std/src/sys_common/net.rs +++ b/library/std/src/sys_common/net.rs @@ -10,7 +10,7 @@ use crate::net::{Ipv4Addr, Ipv6Addr, Shutdown, SocketAddr}; use crate::ptr; use crate::sys::net::netc as c; use crate::sys::net::{cvt, cvt_gai, cvt_r, init, wrlen_t, Socket}; -use crate::sys_common::{AsInner, FromInner, IntoInner}; +use crate::sys_common::{FromInner, IntoInner}; use crate::time::Duration; use libc::{c_int, c_void}; @@ -224,8 +224,8 @@ impl TcpStream { let sock = Socket::new(addr, c::SOCK_STREAM)?; - let (addrp, len) = addr.into_inner(); - cvt_r(|| unsafe { c::connect(sock.as_raw(), addrp, len) })?; + let (addr, len) = addr.into_inner(); + cvt_r(|| unsafe { c::connect(sock.as_raw(), addr.as_ptr(), len) })?; Ok(TcpStream { inner: sock }) } @@ -395,8 +395,8 @@ impl TcpListener { setsockopt(&sock, c::SOL_SOCKET, c::SO_REUSEADDR, 1 as c_int)?; // Bind our new socket - let (addrp, len) = addr.into_inner(); - cvt(unsafe { c::bind(sock.as_raw(), addrp, len as _) })?; + let (addr, len) = addr.into_inner(); + cvt(unsafe { c::bind(sock.as_raw(), addr.as_ptr(), len as _) })?; cfg_if::cfg_if! { if #[cfg(target_os = "horizon")] { @@ -500,8 +500,8 @@ impl UdpSocket { init(); let sock = Socket::new(addr, c::SOCK_DGRAM)?; - let (addrp, len) = addr.into_inner(); - cvt(unsafe { c::bind(sock.as_raw(), addrp, len as _) })?; + let (addr, len) = addr.into_inner(); + cvt(unsafe { c::bind(sock.as_raw(), addr.as_ptr(), len as _) })?; Ok(UdpSocket { inner: sock }) } @@ -531,14 +531,14 @@ impl UdpSocket { pub fn send_to(&self, buf: &[u8], dst: &SocketAddr) -> io::Result<usize> { let len = cmp::min(buf.len(), <wrlen_t>::MAX as usize) as wrlen_t; - let (dstp, dstlen) = dst.into_inner(); + let (dst, dstlen) = dst.into_inner(); let ret = cvt(unsafe { c::sendto( self.inner.as_raw(), buf.as_ptr() as *const c_void, len, MSG_NOSIGNAL, - dstp, + dst.as_ptr(), dstlen, ) })?; @@ -621,7 +621,7 @@ impl UdpSocket { pub fn join_multicast_v6(&self, multiaddr: &Ipv6Addr, interface: u32) -> io::Result<()> { let mreq = c::ipv6_mreq { - ipv6mr_multiaddr: *multiaddr.as_inner(), + ipv6mr_multiaddr: multiaddr.into_inner(), ipv6mr_interface: to_ipv6mr_interface(interface), }; setsockopt(&self.inner, c::IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, mreq) @@ -637,7 +637,7 @@ impl UdpSocket { pub fn leave_multicast_v6(&self, multiaddr: &Ipv6Addr, interface: u32) -> io::Result<()> { let mreq = c::ipv6_mreq { - ipv6mr_multiaddr: *multiaddr.as_inner(), + ipv6mr_multiaddr: multiaddr.into_inner(), ipv6mr_interface: to_ipv6mr_interface(interface), }; setsockopt(&self.inner, c::IPPROTO_IPV6, IPV6_DROP_MEMBERSHIP, mreq) @@ -677,8 +677,8 @@ impl UdpSocket { } pub fn connect(&self, addr: io::Result<&SocketAddr>) -> io::Result<()> { - let (addrp, len) = addr?.into_inner(); - cvt_r(|| unsafe { c::connect(self.inner.as_raw(), addrp, len) }).map(drop) + let (addr, len) = addr?.into_inner(); + cvt_r(|| unsafe { c::connect(self.inner.as_raw(), addr.as_ptr(), len) }).map(drop) } } @@ -700,3 +700,38 @@ impl fmt::Debug for UdpSocket { res.field(name, &self.inner.as_raw()).finish() } } + +//////////////////////////////////////////////////////////////////////////////// +// Converting SocketAddr to libc representation +//////////////////////////////////////////////////////////////////////////////// + +/// A type with the same memory layout as `c::sockaddr`. Used in converting Rust level +/// SocketAddr* types into their system representation. The benefit of this specific +/// type over using `c::sockaddr_storage` is that this type is exactly as large as it +/// needs to be and not a lot larger. And it can be initialized more cleanly from Rust. +#[repr(C)] +pub(crate) union SocketAddrCRepr { + v4: c::sockaddr_in, + v6: c::sockaddr_in6, +} + +impl SocketAddrCRepr { + pub fn as_ptr(&self) -> *const c::sockaddr { + self as *const _ as *const c::sockaddr + } +} + +impl<'a> IntoInner<(SocketAddrCRepr, c::socklen_t)> for &'a SocketAddr { + fn into_inner(self) -> (SocketAddrCRepr, c::socklen_t) { + match *self { + SocketAddr::V4(ref a) => { + let sockaddr = SocketAddrCRepr { v4: a.into_inner() }; + (sockaddr, mem::size_of::<c::sockaddr_in>() as c::socklen_t) + } + SocketAddr::V6(ref a) => { + let sockaddr = SocketAddrCRepr { v6: a.into_inner() }; + (sockaddr, mem::size_of::<c::sockaddr_in6>() as c::socklen_t) + } + } + } +} |
