From 30452d4cda331610418d4dc01023f2d1cdc1ee43 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 4 Aug 2014 09:46:15 -0700 Subject: [PATCH] Change everything returning `libc::sockaddr_storage` to use an &mut out-ptr instead The BSD socket code does some cast tricks with the `libc::sockaddr*` structs, which causes useful data to be stored in struct padding. Since Load/Store instructions do not copy struct padding, this makes these structures dangerous to pass or return by value. In particular, https://github.com/rust-lang/rust/issues/15763 changes return semantics so that a Load instruction is used, breaking the TCP code. Once this PR is merged, that one should merge without error. --- src/libnative/io/net.rs | 35 ++++++++++++++-------------- src/libnative/io/pipe_unix.rs | 22 ++++++++++-------- src/librustuv/net.rs | 43 +++++++++++++++++++---------------- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/src/libnative/io/net.rs b/src/libnative/io/net.rs index cf2d6e7fb45ae..a5332c8d07715 100644 --- a/src/libnative/io/net.rs +++ b/src/libnative/io/net.rs @@ -66,26 +66,27 @@ fn ip_to_inaddr(ip: rtio::IpAddr) -> InAddr { } } -fn addr_to_sockaddr(addr: rtio::SocketAddr) -> (libc::sockaddr_storage, uint) { +fn addr_to_sockaddr(addr: rtio::SocketAddr, + storage: &mut libc::sockaddr_storage) + -> libc::socklen_t { unsafe { - let storage: libc::sockaddr_storage = mem::zeroed(); let len = match ip_to_inaddr(addr.ip) { InAddr(inaddr) => { - let storage: *mut libc::sockaddr_in = mem::transmute(&storage); + let storage = storage as *mut _ as *mut libc::sockaddr_in; (*storage).sin_family = libc::AF_INET as libc::sa_family_t; (*storage).sin_port = htons(addr.port); (*storage).sin_addr = inaddr; mem::size_of::() } In6Addr(inaddr) => { - let storage: *mut libc::sockaddr_in6 = mem::transmute(&storage); + let storage = storage as *mut _ as *mut libc::sockaddr_in6; (*storage).sin6_family = libc::AF_INET6 as libc::sa_family_t; (*storage).sin6_port = htons(addr.port); (*storage).sin6_addr = inaddr; mem::size_of::() } }; - return (storage, len); + return len as libc::socklen_t; } } @@ -277,9 +278,9 @@ impl TcpStream { let fd = try!(socket(addr, libc::SOCK_STREAM)); let ret = TcpStream::new(Inner::new(fd)); - let (addr, len) = addr_to_sockaddr(addr); - let addrp = &addr as *const _ as *const libc::sockaddr; - let len = len as libc::socklen_t; + let mut storage = unsafe { mem::zeroed() }; + let len = addr_to_sockaddr(addr, &mut storage); + let addrp = &storage as *const _ as *const libc::sockaddr; match timeout { Some(timeout) => { @@ -457,9 +458,9 @@ impl TcpListener { let fd = try!(socket(addr, libc::SOCK_STREAM)); let ret = TcpListener { inner: Inner::new(fd) }; - let (addr, len) = addr_to_sockaddr(addr); - let addrp = &addr as *const _ as *const libc::sockaddr; - let len = len as libc::socklen_t; + let mut storage = unsafe { mem::zeroed() }; + let len = addr_to_sockaddr(addr, &mut storage); + let addrp = &storage as *const _ as *const libc::sockaddr; // On platforms with Berkeley-derived sockets, this allows // to quickly rebind a socket, without needing to wait for @@ -566,9 +567,9 @@ impl UdpSocket { write_deadline: 0, }; - let (addr, len) = addr_to_sockaddr(addr); - let addrp = &addr as *const _ as *const libc::sockaddr; - let len = len as libc::socklen_t; + let mut storage = unsafe { mem::zeroed() }; + let len = addr_to_sockaddr(addr, &mut storage); + let addrp = &storage as *const _ as *const libc::sockaddr; match unsafe { libc::bind(fd, addrp, len) } { -1 => Err(last_error()), @@ -656,9 +657,9 @@ impl rtio::RtioUdpSocket for UdpSocket { } fn send_to(&mut self, buf: &[u8], dst: rtio::SocketAddr) -> IoResult<()> { - let (dst, dstlen) = addr_to_sockaddr(dst); - let dstp = &dst as *const _ as *const libc::sockaddr; - let dstlen = dstlen as libc::socklen_t; + let mut storage = unsafe { mem::zeroed() }; + let dstlen = addr_to_sockaddr(dst, &mut storage); + let dstp = &storage as *const _ as *const libc::sockaddr; let fd = self.fd(); let dolock = || self.lock_nonblocking(); diff --git a/src/libnative/io/pipe_unix.rs b/src/libnative/io/pipe_unix.rs index 075ca769d073e..bc3f917a3dcc3 100644 --- a/src/libnative/io/pipe_unix.rs +++ b/src/libnative/io/pipe_unix.rs @@ -29,12 +29,13 @@ fn unix_socket(ty: libc::c_int) -> IoResult { } } -fn addr_to_sockaddr_un(addr: &CString) -> IoResult<(libc::sockaddr_storage, uint)> { +fn addr_to_sockaddr_un(addr: &CString, + storage: &mut libc::sockaddr_storage) + -> IoResult { // the sun_path length is limited to SUN_LEN (with null) assert!(mem::size_of::() >= mem::size_of::()); - let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; - let s: &mut libc::sockaddr_un = unsafe { mem::transmute(&mut storage) }; + let s = unsafe { &mut *(storage as *mut _ as *mut libc::sockaddr_un) }; let len = addr.len(); if len > s.sun_path.len() - 1 { @@ -53,7 +54,7 @@ fn addr_to_sockaddr_un(addr: &CString) -> IoResult<(libc::sockaddr_storage, uint // count the null terminator let len = mem::size_of::() + len + 1; - return Ok((storage, len)); + return Ok(len as libc::socklen_t); } struct Inner { @@ -76,10 +77,10 @@ impl Drop for Inner { fn connect(addr: &CString, ty: libc::c_int, timeout: Option) -> IoResult { - let (addr, len) = try!(addr_to_sockaddr_un(addr)); + let mut storage = unsafe { mem::zeroed() }; + let len = try!(addr_to_sockaddr_un(addr, &mut storage)); let inner = Inner::new(try!(unix_socket(ty))); - let addrp = &addr as *const _ as *const libc::sockaddr; - let len = len as libc::socklen_t; + let addrp = &storage as *const _ as *const libc::sockaddr; match timeout { None => { @@ -96,11 +97,12 @@ fn connect(addr: &CString, ty: libc::c_int, } fn bind(addr: &CString, ty: libc::c_int) -> IoResult { - let (addr, len) = try!(addr_to_sockaddr_un(addr)); + let mut storage = unsafe { mem::zeroed() }; + let len = try!(addr_to_sockaddr_un(addr, &mut storage)); let inner = Inner::new(try!(unix_socket(ty))); - let addrp = &addr as *const _; + let addrp = &storage as *const _ as *const libc::sockaddr; match unsafe { - libc::bind(inner.fd, addrp as *const _, len as libc::socklen_t) + libc::bind(inner.fd, addrp, len) } { -1 => Err(super::last_error()), _ => Ok(inner) diff --git a/src/librustuv/net.rs b/src/librustuv/net.rs index 3cc10ae3823ac..16451f7edd9ab 100644 --- a/src/librustuv/net.rs +++ b/src/librustuv/net.rs @@ -75,17 +75,17 @@ pub fn sockaddr_to_addr(storage: &libc::sockaddr_storage, } } -fn addr_to_sockaddr(addr: rtio::SocketAddr) -> (libc::sockaddr_storage, uint) { +fn addr_to_sockaddr(addr: rtio::SocketAddr, + storage: &mut libc::sockaddr_storage) + -> libc::socklen_t { unsafe { - let mut storage: libc::sockaddr_storage = mem::zeroed(); let len = match addr.ip { rtio::Ipv4Addr(a, b, c, d) => { let ip = (a as u32 << 24) | (b as u32 << 16) | (c as u32 << 8) | (d as u32 << 0); - let storage: &mut libc::sockaddr_in = - mem::transmute(&mut storage); + let storage = storage as *mut _ as *mut libc::sockaddr_in; (*storage).sin_family = libc::AF_INET as libc::sa_family_t; (*storage).sin_port = htons(addr.port); (*storage).sin_addr = libc::in_addr { @@ -95,11 +95,10 @@ fn addr_to_sockaddr(addr: rtio::SocketAddr) -> (libc::sockaddr_storage, uint) { mem::size_of::() } rtio::Ipv6Addr(a, b, c, d, e, f, g, h) => { - let storage: &mut libc::sockaddr_in6 = - mem::transmute(&mut storage); - storage.sin6_family = libc::AF_INET6 as libc::sa_family_t; - storage.sin6_port = htons(addr.port); - storage.sin6_addr = libc::in6_addr { + let storage = storage as *mut _ as *mut libc::sockaddr_in6; + (*storage).sin6_family = libc::AF_INET6 as libc::sa_family_t; + (*storage).sin6_port = htons(addr.port); + (*storage).sin6_addr = libc::in6_addr { s6_addr: [ htons(a), htons(b), @@ -114,7 +113,7 @@ fn addr_to_sockaddr(addr: rtio::SocketAddr) -> (libc::sockaddr_storage, uint) { mem::size_of::() } }; - return (storage, len); + return len as libc::socklen_t } } @@ -203,8 +202,9 @@ impl TcpWatcher { timeout: Option) -> Result { let tcp = TcpWatcher::new(io); let cx = ConnectCtx { status: -1, task: None, timer: None }; - let (addr, _len) = addr_to_sockaddr(address); - let addr_p = &addr as *const _ as *const libc::sockaddr; + let mut storage = unsafe { mem::zeroed() }; + let _len = addr_to_sockaddr(address, &mut storage); + let addr_p = &storage as *const _ as *const libc::sockaddr; cx.connect(tcp, timeout, io, |req, tcp, cb| { unsafe { uvll::uv_tcp_connect(req.handle, tcp.handle, addr_p, cb) } }) @@ -361,10 +361,11 @@ impl TcpListener { outgoing: tx, incoming: rx, }; - let (addr, _len) = addr_to_sockaddr(address); + let mut storage = unsafe { mem::zeroed() }; + let _len = addr_to_sockaddr(address, &mut storage); let res = unsafe { - let addr_p = &addr as *const libc::sockaddr_storage; - uvll::uv_tcp_bind(l.handle, addr_p as *const libc::sockaddr) + let addr_p = &storage as *const _ as *const libc::sockaddr; + uvll::uv_tcp_bind(l.handle, addr_p) }; return match res { 0 => Ok(l.install()), @@ -513,10 +514,11 @@ impl UdpWatcher { assert_eq!(unsafe { uvll::uv_udp_init(io.uv_loop(), udp.handle) }, 0); - let (addr, _len) = addr_to_sockaddr(address); + let mut storage = unsafe { mem::zeroed() }; + let _len = addr_to_sockaddr(address, &mut storage); let result = unsafe { - let addr_p = &addr as *const libc::sockaddr_storage; - uvll::uv_udp_bind(udp.handle, addr_p as *const libc::sockaddr, 0u32) + let addr_p = &storage as *const _ as *const libc::sockaddr; + uvll::uv_udp_bind(udp.handle, addr_p, 0u32) }; return match result { 0 => Ok(udp), @@ -614,8 +616,9 @@ impl rtio::RtioUdpSocket for UdpWatcher { let guard = try!(self.write_access.grant(m)); let mut req = Request::new(uvll::UV_UDP_SEND); - let (addr, _len) = addr_to_sockaddr(dst); - let addr_p = &addr as *const _ as *const libc::sockaddr; + let mut storage = unsafe { mem::zeroed() }; + let _len = addr_to_sockaddr(dst, &mut storage); + let addr_p = &storage as *const _ as *const libc::sockaddr; // see comments in StreamWatcher::write for why we may allocate a buffer // here.