Skip to content

Commit 3405e40

Browse files
committed
Auto merge of rust-lang#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](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.
2 parents e5a7d8f + 73bb371 commit 3405e40

File tree

10 files changed

+172
-247
lines changed

10 files changed

+172
-247
lines changed

library/std/src/net/addr.rs

+63-102
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ use crate::hash;
77
use crate::io::{self, Write};
88
use crate::iter;
99
use crate::mem;
10-
use crate::net::{htons, ntohs, IpAddr, Ipv4Addr, Ipv6Addr};
10+
use crate::net::{IpAddr, Ipv4Addr, Ipv6Addr};
1111
use crate::option;
1212
use crate::slice;
1313
use crate::sys::net::netc as c;
1414
use crate::sys_common::net::LookupHost;
15-
use crate::sys_common::{AsInner, FromInner, IntoInner};
15+
use crate::sys_common::{FromInner, IntoInner};
1616
use crate::vec;
1717

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

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

118118
impl SocketAddr {
@@ -131,7 +131,8 @@ impl SocketAddr {
131131
/// ```
132132
#[stable(feature = "ip_addr", since = "1.7.0")]
133133
#[must_use]
134-
pub fn new(ip: IpAddr, port: u16) -> SocketAddr {
134+
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
135+
pub const fn new(ip: IpAddr, port: u16) -> SocketAddr {
135136
match ip {
136137
IpAddr::V4(a) => SocketAddr::V4(SocketAddrV4::new(a, port)),
137138
IpAddr::V6(a) => SocketAddr::V6(SocketAddrV6::new(a, port, 0, 0)),
@@ -277,15 +278,9 @@ impl SocketAddrV4 {
277278
/// ```
278279
#[stable(feature = "rust1", since = "1.0.0")]
279280
#[must_use]
280-
pub fn new(ip: Ipv4Addr, port: u16) -> SocketAddrV4 {
281-
SocketAddrV4 {
282-
inner: c::sockaddr_in {
283-
sin_family: c::AF_INET as c::sa_family_t,
284-
sin_port: htons(port),
285-
sin_addr: ip.into_inner(),
286-
..unsafe { mem::zeroed() }
287-
},
288-
}
281+
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
282+
pub const fn new(ip: Ipv4Addr, port: u16) -> SocketAddrV4 {
283+
SocketAddrV4 { ip, port }
289284
}
290285

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

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

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

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

@@ -376,17 +369,9 @@ impl SocketAddrV6 {
376369
/// ```
377370
#[stable(feature = "rust1", since = "1.0.0")]
378371
#[must_use]
379-
pub fn new(ip: Ipv6Addr, port: u16, flowinfo: u32, scope_id: u32) -> SocketAddrV6 {
380-
SocketAddrV6 {
381-
inner: c::sockaddr_in6 {
382-
sin6_family: c::AF_INET6 as c::sa_family_t,
383-
sin6_port: htons(port),
384-
sin6_addr: *ip.as_inner(),
385-
sin6_flowinfo: flowinfo,
386-
sin6_scope_id: scope_id,
387-
..unsafe { mem::zeroed() }
388-
},
389-
}
372+
#[rustc_const_unstable(feature = "const_socketaddr", issue = "82485")]
373+
pub const fn new(ip: Ipv6Addr, port: u16, flowinfo: u32, scope_id: u32) -> SocketAddrV6 {
374+
SocketAddrV6 { ip, port, flowinfo, scope_id }
390375
}
391376

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

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

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

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

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

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

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

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

544529
impl FromInner<c::sockaddr_in> for SocketAddrV4 {
545530
fn from_inner(addr: c::sockaddr_in) -> SocketAddrV4 {
546-
SocketAddrV4 { inner: addr }
531+
SocketAddrV4 { ip: Ipv4Addr::from_inner(addr.sin_addr), port: u16::from_be(addr.sin_port) }
547532
}
548533
}
549534

550535
impl FromInner<c::sockaddr_in6> for SocketAddrV6 {
551536
fn from_inner(addr: c::sockaddr_in6) -> SocketAddrV6 {
552-
SocketAddrV6 { inner: addr }
537+
SocketAddrV6 {
538+
ip: Ipv6Addr::from_inner(addr.sin6_addr),
539+
port: u16::from_be(addr.sin6_port),
540+
flowinfo: addr.sin6_flowinfo,
541+
scope_id: addr.sin6_scope_id,
542+
}
543+
}
544+
}
545+
546+
impl IntoInner<c::sockaddr_in> for SocketAddrV4 {
547+
fn into_inner(self) -> c::sockaddr_in {
548+
c::sockaddr_in {
549+
sin_family: c::AF_INET as c::sa_family_t,
550+
sin_port: self.port.to_be(),
551+
sin_addr: self.ip.into_inner(),
552+
..unsafe { mem::zeroed() }
553+
}
554+
}
555+
}
556+
557+
impl IntoInner<c::sockaddr_in6> for SocketAddrV6 {
558+
fn into_inner(self) -> c::sockaddr_in6 {
559+
c::sockaddr_in6 {
560+
sin6_family: c::AF_INET6 as c::sa_family_t,
561+
sin6_port: self.port.to_be(),
562+
sin6_addr: self.ip.into_inner(),
563+
sin6_flowinfo: self.flowinfo,
564+
sin6_scope_id: self.scope_id,
565+
..unsafe { mem::zeroed() }
566+
}
553567
}
554568
}
555569

@@ -582,19 +596,6 @@ impl<I: Into<IpAddr>> From<(I, u16)> for SocketAddr {
582596
}
583597
}
584598

585-
impl<'a> IntoInner<(*const c::sockaddr, c::socklen_t)> for &'a SocketAddr {
586-
fn into_inner(self) -> (*const c::sockaddr, c::socklen_t) {
587-
match *self {
588-
SocketAddr::V4(ref a) => {
589-
(a as *const _ as *const _, mem::size_of_val(a) as c::socklen_t)
590-
}
591-
SocketAddr::V6(ref a) => {
592-
(a as *const _ as *const _, mem::size_of_val(a) as c::socklen_t)
593-
}
594-
}
595-
}
596-
}
597-
598599
#[stable(feature = "rust1", since = "1.0.0")]
599600
impl fmt::Display for SocketAddr {
600601
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -688,40 +689,6 @@ impl fmt::Debug for SocketAddrV6 {
688689
}
689690
}
690691

691-
#[stable(feature = "rust1", since = "1.0.0")]
692-
impl Clone for SocketAddrV4 {
693-
fn clone(&self) -> SocketAddrV4 {
694-
*self
695-
}
696-
}
697-
#[stable(feature = "rust1", since = "1.0.0")]
698-
impl Clone for SocketAddrV6 {
699-
fn clone(&self) -> SocketAddrV6 {
700-
*self
701-
}
702-
}
703-
704-
#[stable(feature = "rust1", since = "1.0.0")]
705-
impl PartialEq for SocketAddrV4 {
706-
fn eq(&self, other: &SocketAddrV4) -> bool {
707-
self.inner.sin_port == other.inner.sin_port
708-
&& self.inner.sin_addr.s_addr == other.inner.sin_addr.s_addr
709-
}
710-
}
711-
#[stable(feature = "rust1", since = "1.0.0")]
712-
impl PartialEq for SocketAddrV6 {
713-
fn eq(&self, other: &SocketAddrV6) -> bool {
714-
self.inner.sin6_port == other.inner.sin6_port
715-
&& self.inner.sin6_addr.s6_addr == other.inner.sin6_addr.s6_addr
716-
&& self.inner.sin6_flowinfo == other.inner.sin6_flowinfo
717-
&& self.inner.sin6_scope_id == other.inner.sin6_scope_id
718-
}
719-
}
720-
#[stable(feature = "rust1", since = "1.0.0")]
721-
impl Eq for SocketAddrV4 {}
722-
#[stable(feature = "rust1", since = "1.0.0")]
723-
impl Eq for SocketAddrV6 {}
724-
725692
#[stable(feature = "socketaddr_ordering", since = "1.45.0")]
726693
impl PartialOrd for SocketAddrV4 {
727694
fn partial_cmp(&self, other: &SocketAddrV4) -> Option<Ordering> {
@@ -753,19 +720,13 @@ impl Ord for SocketAddrV6 {
753720
#[stable(feature = "rust1", since = "1.0.0")]
754721
impl hash::Hash for SocketAddrV4 {
755722
fn hash<H: hash::Hasher>(&self, s: &mut H) {
756-
(self.inner.sin_port, self.inner.sin_addr.s_addr).hash(s)
723+
(self.port, self.ip).hash(s)
757724
}
758725
}
759726
#[stable(feature = "rust1", since = "1.0.0")]
760727
impl hash::Hash for SocketAddrV6 {
761728
fn hash<H: hash::Hasher>(&self, s: &mut H) {
762-
(
763-
self.inner.sin6_port,
764-
&self.inner.sin6_addr.s6_addr,
765-
self.inner.sin6_flowinfo,
766-
self.inner.sin6_scope_id,
767-
)
768-
.hash(s)
729+
(self.port, &self.ip, self.flowinfo, self.scope_id).hash(s)
769730
}
770731
}
771732

0 commit comments

Comments
 (0)