Skip to content

Commit

Permalink
Auto merge of #78802 - faern:simplify-socketaddr, r=joshtriplett
Browse files Browse the repository at this point in the history
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](rust-lang/rfcs#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: tokio-rs/mio#1386.
  - [x] `0.7` branch tokio-rs/mio#1388
  - [x] `0.7.6` published tokio-rs/mio#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: rust-lang/socket2#119.
  - [x] `0.3.x` branch rust-lang/socket2#120
  - [x] `0.3.16` published
  - [x] `master` branch rust-lang/socket2#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: deprecrated/net2-rs#105
  - [x] deprecrated/net2-rs#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: yoshuawuyts/miow#38
  - [x] `0.3.x` - yoshuawuyts/miow#39
  - [x] `0.3.6` published
  - [x] `0.2.x` - yoshuawuyts/miow#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) - quinn-rs/quinn#968 quinn-rs/quinn#987
  - [x] - `quinn 0.6` - quinn-rs/quinn#1045
  - [x] - `quinn 0.5` - quinn-rs/quinn#1046
  - [x] - Release `0.7.0`, `0.6.2` and `0.5.4`
- [x] - `nb-connect` - smol-rs/nb-connect#1
  - [x] - Release `1.0.3`
  - [x] - Yank all versions older than `1.0.3`
- [x] - `shadowsocks-rust` - shadowsocks/shadowsocks-rust#462
- [ ] - `rio` - spacejam/rio#44
- [ ] - `seaslug` - spacejam/seaslug#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.
  • Loading branch information
bors committed Jul 31, 2022
2 parents d9c0e1e + 6b855d5 commit 06e77d2
Show file tree
Hide file tree
Showing 10 changed files with 172 additions and 247 deletions.
165 changes: 63 additions & 102 deletions std/src/net/addr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ use crate::hash;
use crate::io::{self, Write};
use crate::iter;
use crate::mem;
use crate::net::{htons, ntohs, IpAddr, Ipv4Addr, Ipv6Addr};
use crate::net::{IpAddr, Ipv4Addr, Ipv6Addr};
use crate::option;
use crate::slice;
use crate::sys::net::netc as c;
use crate::sys_common::net::LookupHost;
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::sys_common::{FromInner, IntoInner};
use crate::vec;

/// An internet socket address, either IPv4 or IPv6.
Expand Down Expand Up @@ -73,12 +73,11 @@ pub enum SocketAddr {
/// assert_eq!(socket.ip(), &Ipv4Addr::new(127, 0, 0, 1));
/// assert_eq!(socket.port(), 8080);
/// ```
#[derive(Copy)]
#[derive(Copy, Clone, Eq, PartialEq)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct SocketAddrV4 {
// Do not assume that this struct is implemented as the underlying system representation.
// The memory layout is not part of the stable interface that std exposes.
inner: c::sockaddr_in,
ip: Ipv4Addr,
port: u16,
}

/// An IPv6 socket address.
Expand Down Expand Up @@ -107,12 +106,13 @@ pub struct SocketAddrV4 {
/// assert_eq!(socket.ip(), &Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1));
/// assert_eq!(socket.port(), 8080);
/// ```
#[derive(Copy)]
#[derive(Copy, Clone, Eq, PartialEq)]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct SocketAddrV6 {
// Do not assume that this struct is implemented as the underlying system representation.
// The memory layout is not part of the stable interface that std exposes.
inner: c::sockaddr_in6,
ip: Ipv6Addr,
port: u16,
flowinfo: u32,
scope_id: u32,
}

impl SocketAddr {
Expand All @@ -131,7 +131,8 @@ impl SocketAddr {
/// ```
#[stable(feature = "ip_addr", since = "1.7.0")]
#[must_use]
pub fn new(ip: IpAddr, port: u16) -> SocketAddr {
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn new(ip: IpAddr, port: u16) -> SocketAddr {
match ip {
IpAddr::V4(a) => SocketAddr::V4(SocketAddrV4::new(a, port)),
IpAddr::V6(a) => SocketAddr::V6(SocketAddrV6::new(a, port, 0, 0)),
Expand Down Expand Up @@ -277,15 +278,9 @@ impl SocketAddrV4 {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use]
pub fn new(ip: Ipv4Addr, port: u16) -> SocketAddrV4 {
SocketAddrV4 {
inner: c::sockaddr_in {
sin_family: c::AF_INET as c::sa_family_t,
sin_port: htons(port),
sin_addr: ip.into_inner(),
..unsafe { mem::zeroed() }
},
}
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn new(ip: Ipv4Addr, port: u16) -> SocketAddrV4 {
SocketAddrV4 { ip, port }
}

/// Returns the IP address associated with this socket address.
Expand All @@ -302,9 +297,7 @@ impl SocketAddrV4 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn ip(&self) -> &Ipv4Addr {
// SAFETY: `Ipv4Addr` is `#[repr(C)] struct { _: in_addr; }`.
// It is safe to cast from `&in_addr` to `&Ipv4Addr`.
unsafe { &*(&self.inner.sin_addr as *const c::in_addr as *const Ipv4Addr) }
&self.ip
}

/// Changes the IP address associated with this socket address.
Expand All @@ -320,7 +313,7 @@ impl SocketAddrV4 {
/// ```
#[stable(feature = "sockaddr_setters", since = "1.9.0")]
pub fn set_ip(&mut self, new_ip: Ipv4Addr) {
self.inner.sin_addr = new_ip.into_inner()
self.ip = new_ip;
}

/// Returns the port number associated with this socket address.
Expand All @@ -337,7 +330,7 @@ impl SocketAddrV4 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn port(&self) -> u16 {
ntohs(self.inner.sin_port)
self.port
}

/// Changes the port number associated with this socket address.
Expand All @@ -353,7 +346,7 @@ impl SocketAddrV4 {
/// ```
#[stable(feature = "sockaddr_setters", since = "1.9.0")]
pub fn set_port(&mut self, new_port: u16) {
self.inner.sin_port = htons(new_port);
self.port = new_port;
}
}

Expand All @@ -376,17 +369,9 @@ impl SocketAddrV6 {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use]
pub fn new(ip: Ipv6Addr, port: u16, flowinfo: u32, scope_id: u32) -> SocketAddrV6 {
SocketAddrV6 {
inner: c::sockaddr_in6 {
sin6_family: c::AF_INET6 as c::sa_family_t,
sin6_port: htons(port),
sin6_addr: *ip.as_inner(),
sin6_flowinfo: flowinfo,
sin6_scope_id: scope_id,
..unsafe { mem::zeroed() }
},
}
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn new(ip: Ipv6Addr, port: u16, flowinfo: u32, scope_id: u32) -> SocketAddrV6 {
SocketAddrV6 { ip, port, flowinfo, scope_id }
}

/// Returns the IP address associated with this socket address.
Expand All @@ -403,7 +388,7 @@ impl SocketAddrV6 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn ip(&self) -> &Ipv6Addr {
unsafe { &*(&self.inner.sin6_addr as *const c::in6_addr as *const Ipv6Addr) }
&self.ip
}

/// Changes the IP address associated with this socket address.
Expand All @@ -419,7 +404,7 @@ impl SocketAddrV6 {
/// ```
#[stable(feature = "sockaddr_setters", since = "1.9.0")]
pub fn set_ip(&mut self, new_ip: Ipv6Addr) {
self.inner.sin6_addr = *new_ip.as_inner()
self.ip = new_ip;
}

/// Returns the port number associated with this socket address.
Expand All @@ -436,7 +421,7 @@ impl SocketAddrV6 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn port(&self) -> u16 {
ntohs(self.inner.sin6_port)
self.port
}

/// Changes the port number associated with this socket address.
Expand All @@ -452,7 +437,7 @@ impl SocketAddrV6 {
/// ```
#[stable(feature = "sockaddr_setters", since = "1.9.0")]
pub fn set_port(&mut self, new_port: u16) {
self.inner.sin6_port = htons(new_port);
self.port = new_port;
}

/// Returns the flow information associated with this address.
Expand All @@ -479,7 +464,7 @@ impl SocketAddrV6 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn flowinfo(&self) -> u32 {
self.inner.sin6_flowinfo
self.flowinfo
}

/// Changes the flow information associated with this socket address.
Expand All @@ -497,7 +482,7 @@ impl SocketAddrV6 {
/// ```
#[stable(feature = "sockaddr_setters", since = "1.9.0")]
pub fn set_flowinfo(&mut self, new_flowinfo: u32) {
self.inner.sin6_flowinfo = new_flowinfo;
self.flowinfo = new_flowinfo;
}

/// Returns the scope ID associated with this address.
Expand All @@ -519,7 +504,7 @@ impl SocketAddrV6 {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
pub const fn scope_id(&self) -> u32 {
self.inner.sin6_scope_id
self.scope_id
}

/// Changes the scope ID associated with this socket address.
Expand All @@ -537,19 +522,48 @@ impl SocketAddrV6 {
/// ```
#[stable(feature = "sockaddr_setters", since = "1.9.0")]
pub fn set_scope_id(&mut self, new_scope_id: u32) {
self.inner.sin6_scope_id = new_scope_id;
self.scope_id = new_scope_id;
}
}

impl FromInner<c::sockaddr_in> for SocketAddrV4 {
fn from_inner(addr: c::sockaddr_in) -> SocketAddrV4 {
SocketAddrV4 { inner: addr }
SocketAddrV4 { ip: Ipv4Addr::from_inner(addr.sin_addr), port: u16::from_be(addr.sin_port) }
}
}

impl FromInner<c::sockaddr_in6> for SocketAddrV6 {
fn from_inner(addr: c::sockaddr_in6) -> SocketAddrV6 {
SocketAddrV6 { inner: addr }
SocketAddrV6 {
ip: Ipv6Addr::from_inner(addr.sin6_addr),
port: u16::from_be(addr.sin6_port),
flowinfo: addr.sin6_flowinfo,
scope_id: addr.sin6_scope_id,
}
}
}

impl IntoInner<c::sockaddr_in> for SocketAddrV4 {
fn into_inner(self) -> c::sockaddr_in {
c::sockaddr_in {
sin_family: c::AF_INET as c::sa_family_t,
sin_port: self.port.to_be(),
sin_addr: self.ip.into_inner(),
..unsafe { mem::zeroed() }
}
}
}

impl IntoInner<c::sockaddr_in6> for SocketAddrV6 {
fn into_inner(self) -> c::sockaddr_in6 {
c::sockaddr_in6 {
sin6_family: c::AF_INET6 as c::sa_family_t,
sin6_port: self.port.to_be(),
sin6_addr: self.ip.into_inner(),
sin6_flowinfo: self.flowinfo,
sin6_scope_id: self.scope_id,
..unsafe { mem::zeroed() }
}
}
}

Expand Down Expand Up @@ -582,19 +596,6 @@ impl<I: Into<IpAddr>> From<(I, u16)> for SocketAddr {
}
}

impl<'a> IntoInner<(*const c::sockaddr, c::socklen_t)> for &'a SocketAddr {
fn into_inner(self) -> (*const c::sockaddr, c::socklen_t) {
match *self {
SocketAddr::V4(ref a) => {
(a as *const _ as *const _, mem::size_of_val(a) as c::socklen_t)
}
SocketAddr::V6(ref a) => {
(a as *const _ as *const _, mem::size_of_val(a) as c::socklen_t)
}
}
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl fmt::Display for SocketAddr {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down Expand Up @@ -688,40 +689,6 @@ impl fmt::Debug for SocketAddrV6 {
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Clone for SocketAddrV4 {
fn clone(&self) -> SocketAddrV4 {
*self
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl Clone for SocketAddrV6 {
fn clone(&self) -> SocketAddrV6 {
*self
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl PartialEq for SocketAddrV4 {
fn eq(&self, other: &SocketAddrV4) -> bool {
self.inner.sin_port == other.inner.sin_port
&& self.inner.sin_addr.s_addr == other.inner.sin_addr.s_addr
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl PartialEq for SocketAddrV6 {
fn eq(&self, other: &SocketAddrV6) -> bool {
self.inner.sin6_port == other.inner.sin6_port
&& self.inner.sin6_addr.s6_addr == other.inner.sin6_addr.s6_addr
&& self.inner.sin6_flowinfo == other.inner.sin6_flowinfo
&& self.inner.sin6_scope_id == other.inner.sin6_scope_id
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl Eq for SocketAddrV4 {}
#[stable(feature = "rust1", since = "1.0.0")]
impl Eq for SocketAddrV6 {}

#[stable(feature = "socketaddr_ordering", since = "1.45.0")]
impl PartialOrd for SocketAddrV4 {
fn partial_cmp(&self, other: &SocketAddrV4) -> Option<Ordering> {
Expand Down Expand Up @@ -753,19 +720,13 @@ impl Ord for SocketAddrV6 {
#[stable(feature = "rust1", since = "1.0.0")]
impl hash::Hash for SocketAddrV4 {
fn hash<H: hash::Hasher>(&self, s: &mut H) {
(self.inner.sin_port, self.inner.sin_addr.s_addr).hash(s)
(self.port, self.ip).hash(s)
}
}
#[stable(feature = "rust1", since = "1.0.0")]
impl hash::Hash for SocketAddrV6 {
fn hash<H: hash::Hasher>(&self, s: &mut H) {
(
self.inner.sin6_port,
&self.inner.sin6_addr.s6_addr,
self.inner.sin6_flowinfo,
self.inner.sin6_scope_id,
)
.hash(s)
(self.port, &self.ip, self.flowinfo, self.scope_id).hash(s)
}
}

Expand Down
Loading

0 comments on commit 06e77d2

Please sign in to comment.