From 64d7eca0e5a80a961c022eed3581f0ab3f00adfc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 11:59:31 -0800 Subject: [PATCH 1/6] std: Only have extra set_cloexec for files on Linux On Linux we have to do this for binary compatibility with 2.6.18, but for other OSes (e.g. OSX/BSDs/etc) they all support this flag so we don't need to pass it. --- src/libstd/sys/unix/fs.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index e672d9f158666..fc387dbbd471d 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -413,10 +413,18 @@ impl File { libc::open(path.as_ptr(), flags, opts.mode as c_int) })); let fd = FileDesc::new(fd); - // Even though we open with the O_CLOEXEC flag, still set CLOEXEC here, - // in case the open flag is not supported (it's just ignored by the OS - // in that case). - fd.set_cloexec(); + + // Currently the standard library supports Linux 2.6.18 which did not + // have the O_CLOEXEC flag (passed above). If we're running on an older + // Linux kernel then the flag is just ignored by the OS, so we continue + // to explicitly ask for a CLOEXEC fd here. + // + // The CLOEXEC flag, however, is supported on versions of OSX/BSD/etc + // that we support, so we only do this on Linux currently. + if cfg!(target_os = "linux") { + fd.set_cloexec(); + } + Ok(File(fd)) } From 0fff73b64a9658c9a3aa228488964d09e63115b4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 13:22:51 -0800 Subject: [PATCH 2/6] std: When duplicating fds, skip extra set_cloexec Similar to the previous commit, if `F_DUPFD_CLOEXEC` succeeds then there's no need for us to then call `set_cloexec` on platforms other than Linux. The bug mentioned of kernels not actually setting the `CLOEXEC` flag has only been repored on Linux, not elsewhere. --- src/libstd/sys/unix/fd.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index d5f03764be271..0eadee54e2663 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -77,9 +77,7 @@ impl FileDesc { // follow a strategy similar to musl [1] where if passing // F_DUPFD_CLOEXEC causes `fcntl` to return EINVAL it means it's not // supported (the third parameter, 0, is always valid), so we stop - // trying that. We also *still* call the `set_cloexec` method as - // apparently some kernel at some point stopped setting CLOEXEC even - // though it reported doing so on F_DUPFD_CLOEXEC. + // trying that. // // Also note that Android doesn't have F_DUPFD_CLOEXEC, but get it to // resolve so we at least compile this. @@ -95,14 +93,25 @@ impl FileDesc { fd.set_cloexec(); fd }; - static TRY_CLOEXEC: AtomicBool = AtomicBool::new(true); + static TRY_CLOEXEC: AtomicBool = + AtomicBool::new(!cfg!(target_os = "android")); let fd = self.raw(); - if !cfg!(target_os = "android") && TRY_CLOEXEC.load(Ordering::Relaxed) { + if TRY_CLOEXEC.load(Ordering::Relaxed) { match cvt(unsafe { libc::fcntl(fd, F_DUPFD_CLOEXEC, 0) }) { + // We *still* call the `set_cloexec` method as apparently some + // linux kernel at some point stopped setting CLOEXEC even + // though it reported doing so on F_DUPFD_CLOEXEC. + Ok(fd) => { + return Ok(if cfg!(target_os = "linux") { + make_filedesc(fd) + } else { + FileDesc::new(fd) + }) + } Err(ref e) if e.raw_os_error() == Some(libc::EINVAL) => { TRY_CLOEXEC.store(false, Ordering::Relaxed); } - res => return res.map(make_filedesc), + Err(e) => return Err(e), } } cvt(unsafe { libc::fcntl(fd, libc::F_DUPFD, 0) }).map(make_filedesc) From 1bd2d2016178fe85e42d4d34868e9bc58dfd5d07 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 13:30:24 -0800 Subject: [PATCH 3/6] std: Atomically set CLOEXEC for sockets if possible This commit adds support for creating sockets with the `SOCK_CLOEXEC` flag. Support for this flag was added in Linux 2.6.27, however, and support does not exist on platforms other than Linux. For this reason we still have the same fallback as before but just special case Linux if we can. --- src/libstd/sys/unix/net.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/libstd/sys/unix/net.rs b/src/libstd/sys/unix/net.rs index 0aa43803048f3..728bc258c00a8 100644 --- a/src/libstd/sys/unix/net.rs +++ b/src/libstd/sys/unix/net.rs @@ -25,6 +25,16 @@ pub use libc as netc; pub type wrlen_t = size_t; +// See below for the usage of SOCK_CLOEXEC, but this constant is only defined on +// Linux currently (e.g. support doesn't exist on other platforms). In order to +// get name resolution to work and things to compile we just define a dummy +// SOCK_CLOEXEC here for other platforms. Note that the dummy constant isn't +// actually ever used (the blocks below are wrapped in `if cfg!` as well. +#[cfg(target_os = "linux")] +use libc::SOCK_CLOEXEC; +#[cfg(not(target_os = "linux"))] +const SOCK_CLOEXEC: c_int = 0; + pub struct Socket(FileDesc); pub fn init() {} @@ -48,6 +58,19 @@ impl Socket { SocketAddr::V6(..) => libc::AF_INET6, }; unsafe { + // On linux we first attempt to pass the SOCK_CLOEXEC flag to + // atomically create the socket and set it as CLOEXEC. Support for + // this option, however, was added in 2.6.27, and we still support + // 2.6.18 as a kernel, so if the returned error is EINVAL we + // fallthrough to the fallback. + if cfg!(target_os = "linux") { + match cvt(libc::socket(fam, ty | SOCK_CLOEXEC, 0)) { + Ok(fd) => return Ok(Socket(FileDesc::new(fd))), + Err(ref e) if e.raw_os_error() == Some(libc::EINVAL) => {} + Err(e) => return Err(e), + } + } + let fd = try!(cvt(libc::socket(fam, ty, 0))); let fd = FileDesc::new(fd); fd.set_cloexec(); From 1a31e1c09f89daeefa06ca9336912e9bcadb0c1d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 13:56:59 -0800 Subject: [PATCH 4/6] std: Add a helper for symbols that may not exist Right now we only attempt to call one symbol which my not exist everywhere, __pthread_get_minstack, but this pattern will come up more often as we start to bind newer functionality of systems like Linux. Take a similar strategy as the Windows implementation where we use `dlopen` to lookup whether a symbol exists or not. --- src/liblibc | 2 +- src/libstd/sys/unix/mod.rs | 3 ++ src/libstd/sys/unix/thread.rs | 29 +------------ src/libstd/sys/unix/weak.rs | 81 +++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 28 deletions(-) create mode 100644 src/libstd/sys/unix/weak.rs diff --git a/src/liblibc b/src/liblibc index 30f70baa6cc1b..a64ee24718c02 160000 --- a/src/liblibc +++ b/src/liblibc @@ -1 +1 @@ -Subproject commit 30f70baa6cc1ba3ddebb55b988fafbad0c0cc810 +Subproject commit a64ee24718c0289b82a77d692cf56f8a1226de51 diff --git a/src/libstd/sys/unix/mod.rs b/src/libstd/sys/unix/mod.rs index f8a4bcdecd73e..01769a75afde3 100644 --- a/src/libstd/sys/unix/mod.rs +++ b/src/libstd/sys/unix/mod.rs @@ -27,6 +27,9 @@ use ops::Neg; #[cfg(target_os = "openbsd")] pub use os::openbsd as platform; #[cfg(target_os = "solaris")] pub use os::solaris as platform; +#[macro_use] +pub mod weak; + pub mod backtrace; pub mod condvar; pub mod ext; diff --git a/src/libstd/sys/unix/thread.rs b/src/libstd/sys/unix/thread.rs index 277aa5f19f0aa..a7195bab74192 100644 --- a/src/libstd/sys/unix/thread.rs +++ b/src/libstd/sys/unix/thread.rs @@ -317,37 +317,12 @@ pub mod guard { // storage. We need that information to avoid blowing up when a small stack // is created in an application with big thread-local storage requirements. // See #6233 for rationale and details. -// -// Use dlsym to get the symbol value at runtime, both for -// compatibility with older versions of glibc, and to avoid creating -// dependencies on GLIBC_PRIVATE symbols. Assumes that we've been -// dynamically linked to libpthread but that is currently always the -// case. We previously used weak linkage (under the same assumption), -// but that caused Debian to detect an unnecessarily strict versioned -// dependency on libc6 (#23628). #[cfg(target_os = "linux")] #[allow(deprecated)] fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { - use dynamic_lib::DynamicLibrary; - use sync::Once; - - type F = unsafe extern "C" fn(*const libc::pthread_attr_t) -> libc::size_t; - static INIT: Once = Once::new(); - static mut __pthread_get_minstack: Option = None; - - INIT.call_once(|| { - let lib = match DynamicLibrary::open(None) { - Ok(l) => l, - Err(..) => return, - }; - unsafe { - if let Ok(f) = lib.symbol("__pthread_get_minstack") { - __pthread_get_minstack = Some(mem::transmute::<*const (), F>(f)); - } - } - }); + weak!(fn __pthread_get_minstack(*const libc::pthread_attr_t) -> libc::size_t); - match unsafe { __pthread_get_minstack } { + match __pthread_get_minstack.get() { None => libc::PTHREAD_STACK_MIN as usize, Some(f) => unsafe { f(attr) as usize }, } diff --git a/src/libstd/sys/unix/weak.rs b/src/libstd/sys/unix/weak.rs new file mode 100644 index 0000000000000..2cbcd62f53396 --- /dev/null +++ b/src/libstd/sys/unix/weak.rs @@ -0,0 +1,81 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Support for "weak linkage" to symbols on Unix +//! +//! Some I/O operations we do in libstd require newer versions of OSes but we +//! need to maintain binary compatibility with older releases for now. In order +//! to use the new functionality when available we use this module for +//! detection. +//! +//! One option to use here is weak linkage, but that is unfortunately only +//! really workable on Linux. Hence, use dlsym to get the symbol value at +//! runtime. This is also done for compatibility with older versions of glibc, +//! and to avoid creating dependencies on GLIBC_PRIVATE symbols. It assumes that +//! we've been dynamically linked to the library the symbol comes from, but that +//! is currently always the case for things like libpthread/libc. +//! +//! A long time ago this used weak linkage for the __pthread_get_minstack +//! symbol, but that caused Debian to detect an unnecessarily strict versioned +//! dependency on libc6 (#23628). + +use libc; + +use ffi::CString; +use marker; +use mem; +use sync::atomic::{AtomicUsize, Ordering}; + +macro_rules! weak { + (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + static $name: ::sys::weak::Weak $ret> = + ::sys::weak::Weak::new(stringify!($name)); + ) +} + +pub struct Weak { + name: &'static str, + addr: AtomicUsize, + _marker: marker::PhantomData, +} + +impl Weak { + pub const fn new(name: &'static str) -> Weak { + Weak { + name: name, + addr: AtomicUsize::new(1), + _marker: marker::PhantomData, + } + } + + pub fn get(&self) -> Option<&F> { + assert_eq!(mem::size_of::(), mem::size_of::()); + unsafe { + if self.addr.load(Ordering::SeqCst) == 1 { + self.addr.store(fetch(self.name), Ordering::SeqCst); + } + mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr) + } + } +} + +unsafe fn fetch(name: &str) -> usize { + let name = match CString::new(name) { + Ok(cstr) => cstr, + Err(..) => return 0, + }; + let lib = libc::dlopen(0 as *const _, libc::RTLD_LAZY); + if lib.is_null() { + return 0 + } + let ret = libc::dlsym(lib, name.as_ptr()) as usize; + libc::dlclose(lib); + return ret +} From 46315184cb74a98dbd10a0d300a0c3ee62b78049 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 15:22:41 -0800 Subject: [PATCH 5/6] std: Add support for accept4 on Linux This is necessary to atomically accept a socket and set the CLOEXEC flag at the same time. Support only appeared in Linux 2.6.28 so we have to dynamically determine which syscall we're supposed to call in this case. --- src/libstd/sys/unix/net.rs | 26 +++++++++++++++++++++++--- src/libstd/sys/unix/weak.rs | 6 +++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/unix/net.rs b/src/libstd/sys/unix/net.rs index 728bc258c00a8..507cc0f4ea461 100644 --- a/src/libstd/sys/unix/net.rs +++ b/src/libstd/sys/unix/net.rs @@ -12,7 +12,7 @@ use prelude::v1::*; use ffi::CStr; use io; -use libc::{self, c_int, size_t}; +use libc::{self, c_int, size_t, sockaddr, socklen_t}; use net::{SocketAddr, Shutdown}; use str; use sys::fd::FileDesc; @@ -78,8 +78,28 @@ impl Socket { } } - pub fn accept(&self, storage: *mut libc::sockaddr, - len: *mut libc::socklen_t) -> io::Result { + pub fn accept(&self, storage: *mut sockaddr, len: *mut socklen_t) + -> io::Result { + // Unfortunately the only known way right now to accept a socket and + // atomically set the CLOEXEC flag is to use the `accept4` syscall on + // Linux. This was added in 2.6.28, however, and because we support + // 2.6.18 we must detect this support dynamically. + if cfg!(target_os = "linux") { + weak! { + fn accept4(c_int, *mut sockaddr, *mut socklen_t, c_int) -> c_int + } + if let Some(accept) = accept4.get() { + let res = cvt_r(|| unsafe { + accept(self.0.raw(), storage, len, SOCK_CLOEXEC) + }); + match res { + Ok(fd) => return Ok(Socket(FileDesc::new(fd))), + Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => {} + Err(e) => return Err(e), + } + } + } + let fd = try!(cvt_r(|| unsafe { libc::accept(self.0.raw(), storage, len) })); diff --git a/src/libstd/sys/unix/weak.rs b/src/libstd/sys/unix/weak.rs index 2cbcd62f53396..e6f85c08d1246 100644 --- a/src/libstd/sys/unix/weak.rs +++ b/src/libstd/sys/unix/weak.rs @@ -61,7 +61,11 @@ impl Weak { if self.addr.load(Ordering::SeqCst) == 1 { self.addr.store(fetch(self.name), Ordering::SeqCst); } - mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr) + if self.addr.load(Ordering::SeqCst) == 0 { + None + } else { + mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr) + } } } } From 812b309c4791e08aa5bd8dda26c820af43c5fa29 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 15:23:26 -0800 Subject: [PATCH 6/6] std: Try to use pipe2 on Linux for pipes This commit attempts to use the `pipe2` syscall on Linux to atomically set the CLOEXEC flag for pipes created. Unfortunately this was added in 2.6.27 so we have to dynamically determine whether we can use it or not. This commit also updates the `fds-are-cloexec.rs` test to test stdio handles for spawned processes as well. --- src/libstd/sys/unix/pipe.rs | 23 +++++++++++++++++++++-- src/test/run-pass/fds-are-cloexec.rs | 19 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index 5c29c4c08111d..9527b1e2243d3 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -8,9 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use sys::fd::FileDesc; use io; -use libc; +use libc::{self, c_int}; +use sys::cvt_r; +use sys::fd::FileDesc; //////////////////////////////////////////////////////////////////////////////// // Anonymous pipes @@ -20,6 +21,24 @@ pub struct AnonPipe(FileDesc); pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { let mut fds = [0; 2]; + + // Unfortunately the only known way right now to create atomically set the + // CLOEXEC flag is to use the `pipe2` syscall on Linux. This was added in + // 2.6.27, however, and because we support 2.6.18 we must detect this + // support dynamically. + if cfg!(target_os = "linux") { + weak! { fn pipe2(*mut c_int, c_int) -> c_int } + if let Some(pipe) = pipe2.get() { + match cvt_r(|| unsafe { pipe(fds.as_mut_ptr(), libc::O_CLOEXEC) }) { + Ok(_) => { + return Ok((AnonPipe(FileDesc::new(fds[0])), + AnonPipe(FileDesc::new(fds[1])))) + } + Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => {} + Err(e) => return Err(e), + } + } + } if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } { Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1]))) } else { diff --git a/src/test/run-pass/fds-are-cloexec.rs b/src/test/run-pass/fds-are-cloexec.rs index 3be47e8430d59..3c7d2861c877e 100644 --- a/src/test/run-pass/fds-are-cloexec.rs +++ b/src/test/run-pass/fds-are-cloexec.rs @@ -16,11 +16,11 @@ extern crate libc; use std::env; -use std::fs::{self, File}; +use std::fs::File; use std::io; use std::net::{TcpListener, TcpStream, UdpSocket}; use std::os::unix::prelude::*; -use std::process::Command; +use std::process::{Command, Stdio}; use std::thread; fn main() { @@ -45,6 +45,17 @@ fn parent() { let udp1 = UdpSocket::bind("127.0.0.1:0").unwrap(); let udp2 = udp1.try_clone().unwrap(); + let mut child = Command::new(env::args().next().unwrap()) + .arg("100") + .stdout(Stdio::piped()) + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn().unwrap(); + let pipe1 = child.stdin.take().unwrap(); + let pipe2 = child.stdout.take().unwrap(); + let pipe3 = child.stderr.take().unwrap(); + + let status = Command::new(env::args().next().unwrap()) .arg(file.as_raw_fd().to_string()) .arg(tcp1.as_raw_fd().to_string()) @@ -55,9 +66,13 @@ fn parent() { .arg(tcp6.as_raw_fd().to_string()) .arg(udp1.as_raw_fd().to_string()) .arg(udp2.as_raw_fd().to_string()) + .arg(pipe1.as_raw_fd().to_string()) + .arg(pipe2.as_raw_fd().to_string()) + .arg(pipe3.as_raw_fd().to_string()) .status() .unwrap(); assert!(status.success()); + child.wait().unwrap(); } fn child(args: &[String]) {