Auto merge of #24034 - alexcrichton:cloexec, r=aturon

The commit messages have more details as to what's going on, but this is a breaking change for any libraries which expect file descriptors to be inherited by default.

Closes #12148

8 files changed, 304 insertions, 298 deletions

diff --git a/src/libstd/sys/unix/c.rs b/src/libstd/sys/unix/c.rs
index 5ae508e4610..aa4bf821207 100644
--- a/src/libstd/sys/unix/c.rs
+++ b/src/libstd/sys/unix/c.rs
@@ -26,39 +26,35 @@ use libc;
           target_os = "dragonfly",
           target_os = "bitrig",
           target_os = "openbsd"))]
-pub const FIONBIO: libc::c_ulong = 0x8004667e;
-#[cfg(any(all(target_os = "linux",
-              any(target_arch = "x86",
-                  target_arch = "x86_64",
-                  target_arch = "arm",
-                  target_arch = "aarch64")),
-          target_os = "android"))]
-pub const FIONBIO: libc::c_ulong = 0x5421;
-#[cfg(all(target_os = "linux",
-          any(target_arch = "mips",
-              target_arch = "mipsel",
-              target_arch = "powerpc")))]
-pub const FIONBIO: libc::c_ulong = 0x667e;
-
-#[cfg(any(target_os = "macos",
-          target_os = "ios",
-          target_os = "freebsd",
-          target_os = "dragonfly",
-          target_os = "bitrig",
-          target_os = "openbsd"))]
-pub const FIOCLEX: libc::c_ulong = 0x20006601;
+mod consts {
+    use libc;
+    pub const FIONBIO: libc::c_ulong = 0x8004667e;
+    pub const FIOCLEX: libc::c_ulong = 0x20006601;
+    pub const FIONCLEX: libc::c_ulong = 0x20006602;
+}
 #[cfg(any(all(target_os = "linux",
               any(target_arch = "x86",
                   target_arch = "x86_64",
                   target_arch = "arm",
                   target_arch = "aarch64")),
           target_os = "android"))]
-pub const FIOCLEX: libc::c_ulong = 0x5451;
+mod consts {
+    use libc;
+    pub const FIONBIO: libc::c_ulong = 0x5421;
+    pub const FIOCLEX: libc::c_ulong = 0x5451;
+    pub const FIONCLEX: libc::c_ulong = 0x5450;
+}
 #[cfg(all(target_os = "linux",
           any(target_arch = "mips",
               target_arch = "mipsel",
               target_arch = "powerpc")))]
-pub const FIOCLEX: libc::c_ulong = 0x6601;
+mod consts {
+    use libc;
+    pub const FIONBIO: libc::c_ulong = 0x667e;
+    pub const FIOCLEX: libc::c_ulong = 0x6601;
+    pub const FIONCLEX: libc::c_ulong = 0x6600;
+}
+pub use self::consts::*;
 
 #[cfg(any(target_os = "macos",
           target_os = "ios",
@@ -163,6 +159,8 @@ extern {
     pub fn utimes(filename: *const libc::c_char,
                   times: *const libc::timeval) -> libc::c_int;
     pub fn gai_strerror(errcode: libc::c_int) -> *const libc::c_char;
+    pub fn setgroups(ngroups: libc::c_int,
+                     ptr: *const libc::c_void) -> libc::c_int;
 }
 
 #[cfg(any(target_os = "macos", target_os = "ios"))]
diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs
index f7c57c3f5e5..d86c77624e8 100644
--- a/src/libstd/sys/unix/fd.rs
+++ b/src/libstd/sys/unix/fd.rs
@@ -13,6 +13,7 @@ use core::prelude::*;
 use io;
 use libc::{self, c_int, size_t, c_void};
 use mem;
+use sys::c;
 use sys::cvt;
 use sys_common::AsInner;
 
@@ -51,6 +52,13 @@ impl FileDesc {
         }));
         Ok(ret as usize)
     }
+
+    pub fn set_cloexec(&self) {
+        unsafe {
+            let ret = c::ioctl(self.fd, c::FIOCLEX);
+            debug_assert_eq!(ret, 0);
+        }
+    }
 }
 
 impl AsInner<c_int> for FileDesc {
@@ -59,14 +67,11 @@ impl AsInner<c_int> for FileDesc {
 
 impl Drop for FileDesc {
     fn drop(&mut self) {
-        // closing stdio file handles makes no sense, so never do it. Also, note
-        // that errors are ignored when closing a file descriptor. The reason
-        // for this is that if an error occurs we don't actually know if the
-        // file descriptor was closed or not, and if we retried (for something
-        // like EINTR), we might close another valid file descriptor (opened
-        // after we closed ours.
-        if self.fd > libc::STDERR_FILENO {
-            let _ = unsafe { libc::close(self.fd) };
-        }
+        // Note that errors are ignored when closing a file descriptor. The
+        // reason for this is that if an error occurs we don't actually know if
+        // the file descriptor was closed or not, and if we retried (for
+        // something like EINTR), we might close another valid file descriptor
+        // (opened after we closed ours.
+        let _ = unsafe { libc::close(self.fd) };
     }
 }
diff --git a/src/libstd/sys/unix/fs2.rs b/src/libstd/sys/unix/fs2.rs
index c0426af051b..ac121f1c82e 100644
--- a/src/libstd/sys/unix/fs2.rs
+++ b/src/libstd/sys/unix/fs2.rs
@@ -205,19 +205,27 @@ impl OpenOptions {
 
 impl File {
     pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
+        let path = try!(cstr(path));
+        File::open_c(&path, opts)
+    }
+
+    pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
         let flags = opts.flags | match (opts.read, opts.write) {
             (true, true) => libc::O_RDWR,
             (false, true) => libc::O_WRONLY,
             (true, false) |
             (false, false) => libc::O_RDONLY,
         };
-        let path = try!(cstr(path));
         let fd = try!(cvt_r(|| unsafe {
             libc::open(path.as_ptr(), flags, opts.mode)
         }));
-        Ok(File(FileDesc::new(fd)))
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        Ok(File(fd))
     }
 
+    pub fn into_fd(self) -> FileDesc { self.0 }
+
     pub fn file_attr(&self) -> io::Result<FileAttr> {
         let mut stat: libc::stat = unsafe { mem::zeroed() };
         try!(cvt(unsafe { libc::fstat(self.0.raw(), &mut stat) }));
diff --git a/src/libstd/sys/unix/net.rs b/src/libstd/sys/unix/net.rs
index 908136a42ab..2e1cbb2a1e1 100644
--- a/src/libstd/sys/unix/net.rs
+++ b/src/libstd/sys/unix/net.rs
@@ -47,7 +47,9 @@ impl Socket {
         };
         unsafe {
             let fd = try!(cvt(libc::socket(fam, ty, 0)));
-            Ok(Socket(FileDesc::new(fd)))
+            let fd = FileDesc::new(fd);
+            fd.set_cloexec();
+            Ok(Socket(fd))
         }
     }
 
@@ -56,13 +58,16 @@ impl Socket {
         let fd = try!(cvt_r(|| unsafe {
             libc::accept(self.0.raw(), storage, len)
         }));
-        Ok(Socket(FileDesc::new(fd)))
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        Ok(Socket(fd))
     }
 
     pub fn duplicate(&self) -> io::Result<Socket> {
-        cvt(unsafe { libc::dup(self.0.raw()) }).map(|fd| {
-            Socket(FileDesc::new(fd))
-        })
+        let fd = try!(cvt(unsafe { libc::dup(self.0.raw()) }));
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        Ok(Socket(fd))
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
diff --git a/src/libstd/sys/unix/pipe2.rs b/src/libstd/sys/unix/pipe2.rs
index 7af2c0f0b2a..e9d8c69fefb 100644
--- a/src/libstd/sys/unix/pipe2.rs
+++ b/src/libstd/sys/unix/pipe2.rs
@@ -20,11 +20,10 @@ use libc;
 
 pub struct AnonPipe(FileDesc);
 
-pub unsafe fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
+pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
     let mut fds = [0; 2];
-    if libc::pipe(fds.as_mut_ptr()) == 0 {
-        Ok((AnonPipe::from_fd(fds[0]),
-            AnonPipe::from_fd(fds[1])))
+    if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } {
+        Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
     } else {
         Err(io::Error::last_os_error())
     }
@@ -32,7 +31,9 @@ pub unsafe fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
 
 impl AnonPipe {
     pub fn from_fd(fd: libc::c_int) -> AnonPipe {
-        AnonPipe(FileDesc::new(fd))
+        let fd = FileDesc::new(fd);
+        fd.set_cloexec();
+        AnonPipe(fd)
     }
 
     pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
@@ -43,7 +44,7 @@ impl AnonPipe {
         self.0.write(buf)
     }
 
-    pub fn raw(&self) -> libc::c_int {
-        self.0.raw()
+    pub fn into_fd(self) -> FileDesc {
+        self.0
     }
 }
diff --git a/src/libstd/sys/unix/process2.rs b/src/libstd/sys/unix/process2.rs
index 60f00c80b4a..0b4e871454d 100644
--- a/src/libstd/sys/unix/process2.rs
+++ b/src/libstd/sys/unix/process2.rs
@@ -13,14 +13,14 @@ use os::unix::prelude::*;
 
 use collections::HashMap;
 use env;
-use ffi::{OsString, OsStr, CString};
+use ffi::{OsString, OsStr, CString, CStr};
 use fmt;
 use io::{self, Error, ErrorKind};
 use libc::{self, pid_t, c_void, c_int, gid_t, uid_t};
-use mem;
 use ptr;
 use sys::pipe2::AnonPipe;
 use sys::{self, retry, c, cvt};
+use sys::fs2::{File, OpenOptions};
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
@@ -119,6 +119,12 @@ pub struct Process {
     pid: pid_t
 }
 
+pub enum Stdio {
+    Inherit,
+    Piped(AnonPipe),
+    None,
+}
+
 const CLOEXEC_MSG_FOOTER: &'static [u8] = b"NOEX";
 
 impl Process {
@@ -128,221 +134,185 @@ impl Process {
     }
 
     pub fn spawn(cfg: &Command,
-                 in_fd: Option<AnonPipe>, out_fd: Option<AnonPipe>, err_fd: Option<AnonPipe>)
-                 -> io::Result<Process>
-    {
-        use libc::funcs::posix88::unistd::{fork, dup2, close, chdir, execvp};
-
-        mod rustrt {
-            extern {
-                pub fn rust_unset_sigprocmask();
+                 in_fd: Stdio,
+                 out_fd: Stdio,
+                 err_fd: Stdio) -> io::Result<Process> {
+        let dirp = cfg.cwd.as_ref().map(|c| c.as_ptr()).unwrap_or(ptr::null());
+
+        let (envp, _a, _b) = make_envp(cfg.env.as_ref());
+        let (argv, _a) = make_argv(&cfg.program, &cfg.args);
+        let (input, output) = try!(sys::pipe2::anon_pipe());
+
+        let pid = unsafe {
+            match libc::fork() {
+                0 => {
+                    drop(input);
+                    Process::child_after_fork(cfg, output, argv, envp, dirp,
+                                              in_fd, out_fd, err_fd)
+                }
+                n if n < 0 => return Err(Error::last_os_error()),
+                n => n,
+            }
+        };
+
+        let p = Process{ pid: pid };
+        drop(output);
+        let mut bytes = [0; 8];
+
+        // loop to handle EINTR
+        loop {
+            match input.read(&mut bytes) {
+                Ok(0) => return Ok(p),
+                Ok(8) => {
+                    assert!(combine(CLOEXEC_MSG_FOOTER) == combine(&bytes[4.. 8]),
+                            "Validation on the CLOEXEC pipe failed: {:?}", bytes);
+                    let errno = combine(&bytes[0.. 4]);
+                    assert!(p.wait().is_ok(),
+                            "wait() should either return Ok or panic");
+                    return Err(Error::from_raw_os_error(errno))
+                }
+                Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
+                Err(e) => {
+                    assert!(p.wait().is_ok(),
+                            "wait() should either return Ok or panic");
+                    panic!("the CLOEXEC pipe failed: {:?}", e)
+                },
+                Ok(..) => { // pipe I/O up to PIPE_BUF bytes should be atomic
+                    assert!(p.wait().is_ok(),
+                            "wait() should either return Ok or panic");
+                    panic!("short read on the CLOEXEC pipe")
+                }
             }
         }
 
-        unsafe fn set_cloexec(fd: c_int) {
-            let ret = c::ioctl(fd, c::FIOCLEX);
-            assert_eq!(ret, 0);
-        }
+        fn combine(arr: &[u8]) -> i32 {
+            let a = arr[0] as u32;
+            let b = arr[1] as u32;
+            let c = arr[2] as u32;
+            let d = arr[3] as u32;
 
-        #[cfg(all(target_os = "android", target_arch = "aarch64"))]
-        unsafe fn getdtablesize() -> c_int {
-            libc::sysconf(libc::consts::os::sysconf::_SC_OPEN_MAX) as c_int
+            ((a << 24) | (b << 16) | (c << 8) | (d << 0)) as i32
         }
+    }
 
-        #[cfg(not(all(target_os = "android", target_arch = "aarch64")))]
-        unsafe fn getdtablesize() -> c_int {
-            libc::funcs::bsd44::getdtablesize()
+    // And at this point we've reached a special time in the life of the
+    // child. The child must now be considered hamstrung and unable to
+    // do anything other than syscalls really. Consider the following
+    // scenario:
+    //
+    //      1. Thread A of process 1 grabs the malloc() mutex
+    //      2. Thread B of process 1 forks(), creating thread C
+    //      3. Thread C of process 2 then attempts to malloc()
+    //      4. The memory of process 2 is the same as the memory of
+    //         process 1, so the mutex is locked.
+    //
+    // This situation looks a lot like deadlock, right? It turns out
+    // that this is what pthread_atfork() takes care of, which is
+    // presumably implemented across platforms. The first thing that
+    // threads to *before* forking is to do things like grab the malloc
+    // mutex, and then after the fork they unlock it.
+    //
+    // Despite this information, libnative's spawn has been witnessed to
+    // deadlock on both OSX and FreeBSD. I'm not entirely sure why, but
+    // all collected backtraces point at malloc/free traffic in the
+    // child spawned process.
+    //
+    // For this reason, the block of code below should contain 0
+    // invocations of either malloc of free (or their related friends).
+    //
+    // As an example of not having malloc/free traffic, we don't close
+    // this file descriptor by dropping the FileDesc (which contains an
+    // allocation). Instead we just close it manually. This will never
+    // have the drop glue anyway because this code never returns (the
+    // child will either exec() or invoke libc::exit)
+    unsafe fn child_after_fork(cfg: &Command,
+                               mut output: AnonPipe,
+                               argv: *const *const libc::c_char,
+                               envp: *const libc::c_void,
+                               dirp: *const libc::c_char,
+                               in_fd: Stdio,
+                               out_fd: Stdio,
+                               err_fd: Stdio) -> ! {
+        fn fail(output: &mut AnonPipe) -> ! {
+            let errno = sys::os::errno() as u32;
+            let bytes = [
+                (errno >> 24) as u8,
+                (errno >> 16) as u8,
+                (errno >>  8) as u8,
+                (errno >>  0) as u8,
+                CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1],
+                CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3]
+            ];
+            // pipe I/O up to PIPE_BUF bytes should be atomic, and then we want
+            // to be sure we *don't* run at_exit destructors as we're being torn
+            // down regardless
+            assert!(output.write(&bytes).is_ok());
+            unsafe { libc::_exit(1) }
         }
 
-        let dirp = cfg.cwd.as_ref().map(|c| c.as_ptr()).unwrap_or(ptr::null());
-
-        with_envp(cfg.env.as_ref(), |envp: *const c_void| {
-            with_argv(&cfg.program, &cfg.args, |argv: *const *const libc::c_char| unsafe {
-                let (input, mut output) = try!(sys::pipe2::anon_pipe());
-
-                // We may use this in the child, so perform allocations before the
-                // fork
-                let devnull = b"/dev/null\0";
-
-                set_cloexec(output.raw());
-
-                let pid = fork();
-                if pid < 0 {
-                    return Err(Error::last_os_error())
-                } else if pid > 0 {
-                    #[inline]
-                    fn combine(arr: &[u8]) -> i32 {
-                        let a = arr[0] as u32;
-                        let b = arr[1] as u32;
-                        let c = arr[2] as u32;
-                        let d = arr[3] as u32;
-
-                        ((a << 24) | (b << 16) | (c << 8) | (d << 0)) as i32
-                    }
-
-                    let p = Process{ pid: pid };
-                    drop(output);
-                    let mut bytes = [0; 8];
-
-                    // loop to handle EINTER
-                    loop {
-                        match input.read(&mut bytes) {
-                            Ok(8) => {
-                                assert!(combine(CLOEXEC_MSG_FOOTER) == combine(&bytes[4.. 8]),
-                                        "Validation on the CLOEXEC pipe failed: {:?}", bytes);
-                                let errno = combine(&bytes[0.. 4]);
-                                assert!(p.wait().is_ok(),
-                                        "wait() should either return Ok or panic");
-                                return Err(Error::from_raw_os_error(errno))
-                            }
-                            Ok(0) => return Ok(p),
-                            Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
-                            Err(e) => {
-                                assert!(p.wait().is_ok(),
-                                        "wait() should either return Ok or panic");
-                                panic!("the CLOEXEC pipe failed: {:?}", e)
-                            },
-                            Ok(..) => { // pipe I/O up to PIPE_BUF bytes should be atomic
-                                assert!(p.wait().is_ok(),
-                                        "wait() should either return Ok or panic");
-                                panic!("short read on the CLOEXEC pipe")
-                            }
-                        }
+        let setup = |src: Stdio, dst: c_int| {
+            let fd = match src {
+                Stdio::Inherit => return true,
+                Stdio::Piped(pipe) => pipe.into_fd(),
+
+                // If a stdio file descriptor is set to be ignored, we open up
+                // /dev/null into that file descriptor. Otherwise, the first
+                // file descriptor opened up in the child would be numbered as
+                // one of the stdio file descriptors, which is likely to wreak
+                // havoc.
+                Stdio::None => {
+                    let mut opts = OpenOptions::new();
+                    opts.read(dst == libc::STDIN_FILENO);
+                    opts.write(dst != libc::STDIN_FILENO);
+                    let devnull = CStr::from_ptr(b"/dev/null\0".as_ptr()
+                                                    as *const _);
+                    if let Ok(f) = File::open_c(devnull, &opts) {
+                        f.into_fd()
+                    } else {
+                        return false
                     }
                 }
+            };
+            retry(|| libc::dup2(fd.raw(), dst)) != -1
+        };
 
-                // And at this point we've reached a special time in the life of the
-                // child. The child must now be considered hamstrung and unable to
-                // do anything other than syscalls really. Consider the following
-                // scenario:
-                //
-                //      1. Thread A of process 1 grabs the malloc() mutex
-                //      2. Thread B of process 1 forks(), creating thread C
-                //      3. Thread C of process 2 then attempts to malloc()
-                //      4. The memory of process 2 is the same as the memory of
-                //         process 1, so the mutex is locked.
-                //
-                // This situation looks a lot like deadlock, right? It turns out
-                // that this is what pthread_atfork() takes care of, which is
-                // presumably implemented across platforms. The first thing that
-                // threads to *before* forking is to do things like grab the malloc
-                // mutex, and then after the fork they unlock it.
-                //
-                // Despite this information, libnative's spawn has been witnessed to
-                // deadlock on both OSX and FreeBSD. I'm not entirely sure why, but
-                // all collected backtraces point at malloc/free traffic in the
-                // child spawned process.
-                //
-                // For this reason, the block of code below should contain 0
-                // invocations of either malloc of free (or their related friends).
-                //
-                // As an example of not having malloc/free traffic, we don't close
-                // this file descriptor by dropping the FileDesc (which contains an
-                // allocation). Instead we just close it manually. This will never
-                // have the drop glue anyway because this code never returns (the
-                // child will either exec() or invoke libc::exit)
-                let _ = libc::close(input.raw());
-
-                fn fail(output: &mut AnonPipe) -> ! {
-                    let errno = sys::os::errno() as u32;
-                    let bytes = [
-                        (errno >> 24) as u8,
-                        (errno >> 16) as u8,
-                        (errno >>  8) as u8,
-                        (errno >>  0) as u8,
-                        CLOEXEC_MSG_FOOTER[0], CLOEXEC_MSG_FOOTER[1],
-                        CLOEXEC_MSG_FOOTER[2], CLOEXEC_MSG_FOOTER[3]
-                    ];
-                    // pipe I/O up to PIPE_BUF bytes should be atomic
-                    assert!(output.write(&bytes).is_ok());
-                    unsafe { libc::_exit(1) }
-                }
-
-                rustrt::rust_unset_sigprocmask();
-
-                // If a stdio file descriptor is set to be ignored, we don't
-                // actually close it, but rather open up /dev/null into that
-                // file descriptor. Otherwise, the first file descriptor opened
-                // up in the child would be numbered as one of the stdio file
-                // descriptors, which is likely to wreak havoc.
-                let setup = |src: Option<AnonPipe>, dst: c_int| {
-                    let src = match src {
-                        None => {
-                            let flags = if dst == libc::STDIN_FILENO {
-                                libc::O_RDONLY
-                            } else {
-                                libc::O_RDWR
-                            };
-                            libc::open(devnull.as_ptr() as *const _, flags, 0)
-                        }
-                        Some(obj) => {
-                            let fd = obj.raw();
-                            // Leak the memory and the file descriptor. We're in the
-                            // child now an all our resources are going to be
-                            // cleaned up very soon
-                            mem::forget(obj);
-                            fd
-                        }
-                    };
-                    src != -1 && retry(|| dup2(src, dst)) != -1
-                };
-
-                if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
-                if !setup(out_fd, libc::STDOUT_FILENO) { fail(&mut output) }
-                if !setup(err_fd, libc::STDERR_FILENO) { fail(&mut output) }
-
-                // close all other fds
-                for fd in (3..getdtablesize()).rev() {
-                    if fd != output.raw() {
-                        let _ = close(fd as c_int);
-                    }
-                }
+        if !setup(in_fd, libc::STDIN_FILENO) { fail(&mut output) }
+        if !setup(out_fd, libc::STDOUT_FILENO) { fail(&mut output) }
+        if !setup(err_fd, libc::STDERR_FILENO) { fail(&mut output) }
 
-                match cfg.gid {
-                    Some(u) => {
-                        if libc::setgid(u as libc::gid_t) != 0 {
-                            fail(&mut output);
-                        }
-                    }
-                    None => {}
-                }
-                match cfg.uid {
-                    Some(u) => {
-                        // When dropping privileges from root, the `setgroups` call
-                        // will remove any extraneous groups. If we don't call this,
-                        // then even though our uid has dropped, we may still have
-                        // groups that enable us to do super-user things. This will
-                        // fail if we aren't root, so don't bother checking the
-                        // return value, this is just done as an optimistic
-                        // privilege dropping function.
-                        extern {
-                            fn setgroups(ngroups: libc::c_int,
-                                         ptr: *const libc::c_void) -> libc::c_int;
-                        }
-                        let _ = setgroups(0, ptr::null());
-
-                        if libc::setuid(u as libc::uid_t) != 0 {
-                            fail(&mut output);
-                        }
-                    }
-                    None => {}
-                }
-                if cfg.detach {
-                    // Don't check the error of setsid because it fails if we're the
-                    // process leader already. We just forked so it shouldn't return
-                    // error, but ignore it anyway.
-                    let _ = libc::setsid();
-                }
-                if !dirp.is_null() && chdir(dirp) == -1 {
-                    fail(&mut output);
-                }
-                if !envp.is_null() {
-                    *sys::os::environ() = envp as *const _;
-                }
-                let _ = execvp(*argv, argv as *mut _);
+        if let Some(u) = cfg.gid {
+            if libc::setgid(u as libc::gid_t) != 0 {
                 fail(&mut output);
-            })
-        })
+            }
+        }
+        if let Some(u) = cfg.uid {
+            // When dropping privileges from root, the `setgroups` call
+            // will remove any extraneous groups. If we don't call this,
+            // then even though our uid has dropped, we may still have
+            // groups that enable us to do super-user things. This will
+            // fail if we aren't root, so don't bother checking the
+            // return value, this is just done as an optimistic
+            // privilege dropping function.
+            let _ = c::setgroups(0, ptr::null());
+
+            if libc::setuid(u as libc::uid_t) != 0 {
+                fail(&mut output);
+            }
+        }
+        if cfg.detach {
+            // Don't check the error of setsid because it fails if we're the
+            // process leader already. We just forked so it shouldn't return
+            // error, but ignore it anyway.
+            let _ = libc::setsid();
+        }
+        if !dirp.is_null() && libc::chdir(dirp) == -1 {
+            fail(&mut output);
+        }
+        if !envp.is_null() {
+            *sys::os::environ() = envp as *const _;
+        }
+        let _ = libc::execvp(*argv, argv as *mut _);
+        fail(&mut output)
     }
 
     pub fn wait(&self) -> io::Result<ExitStatus> {
@@ -364,8 +334,8 @@ impl Process {
     }
 }
 
-fn with_argv<T,F>(prog: &CString, args: &[CString], cb: F) -> T
-    where F : FnOnce(*const *const libc::c_char) -> T
+fn make_argv(prog: &CString, args: &[CString])
+             -> (*const *const libc::c_char, Vec<*const libc::c_char>)
 {
     let mut ptrs: Vec<*const libc::c_char> = Vec::with_capacity(args.len()+1);
 
@@ -380,40 +350,38 @@ fn with_argv<T,F>(prog: &CString, args: &[CString], cb: F) -> T
     // Add a terminating null pointer (required by libc).
     ptrs.push(ptr::null());
 
-    cb(ptrs.as_ptr())
+    (ptrs.as_ptr(), ptrs)
 }
 
-fn with_envp<T, F>(env: Option<&HashMap<OsString, OsString>>, cb: F) -> T
-    where F : FnOnce(*const c_void) -> T
+fn make_envp(env: Option<&HashMap<OsString, OsString>>)
+             -> (*const c_void, Vec<Vec<u8>>, Vec<*const libc::c_char>)
 {
     // On posixy systems we can pass a char** for envp, which is a
     // null-terminated array of "k=v\0" strings. Since we must create
     // these strings locally, yet expose a raw pointer to them, we
     // create a temporary vector to own the CStrings that outlives the
     // call to cb.
-    match env {
-        Some(env) => {
-            let mut tmps = Vec::with_capacity(env.len());
-
-            for pair in env {
-                let mut kv = Vec::new();
-                kv.push_all(pair.0.as_bytes());
-                kv.push('=' as u8);
-                kv.push_all(pair.1.as_bytes());
-                kv.push(0); // terminating null
-                tmps.push(kv);
-            }
+    if let Some(env) = env {
+        let mut tmps = Vec::with_capacity(env.len());
+
+        for pair in env {
+            let mut kv = Vec::new();
+            kv.push_all(pair.0.as_bytes());
+            kv.push('=' as u8);
+            kv.push_all(pair.1.as_bytes());
+            kv.push(0); // terminating null
+            tmps.push(kv);
+        }
 
-            // As with `with_argv`, this is unsafe, since cb could leak the pointers.
-            let mut ptrs: Vec<*const libc::c_char> =
-                tmps.iter()
-                    .map(|tmp| tmp.as_ptr() as *const libc::c_char)
-                    .collect();
-            ptrs.push(ptr::null());
+        let mut ptrs: Vec<*const libc::c_char> =
+            tmps.iter()
+                .map(|tmp| tmp.as_ptr() as *const libc::c_char)
+                .collect();
+        ptrs.push(ptr::null());
 
-            cb(ptrs.as_ptr() as *const c_void)
-        }
-        _ => cb(ptr::null())
+        (ptrs.as_ptr() as *const _, tmps, ptrs)
+    } else {
+        (0 as *const _, Vec::new(), Vec::new())
     }
 }
 
diff --git a/src/libstd/sys/windows/pipe2.rs b/src/libstd/sys/windows/pipe2.rs
index 229481e3d57..ed41c959782 100644
--- a/src/libstd/sys/windows/pipe2.rs
+++ b/src/libstd/sys/windows/pipe2.rs
@@ -22,22 +22,24 @@ pub struct AnonPipe {
     fd: c_int
 }
 
-pub unsafe fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
+pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> {
     // Windows pipes work subtly differently than unix pipes, and their
     // inheritance has to be handled in a different way that I do not
     // fully understand. Here we explicitly make the pipe non-inheritable,
     // which means to pass it to a subprocess they need to be duplicated
     // first, as in std::run.
     let mut fds = [0; 2];
-    match libc::pipe(fds.as_mut_ptr(), 1024 as ::libc::c_uint,
-    (libc::O_BINARY | libc::O_NOINHERIT) as c_int) {
-        0 => {
-            assert!(fds[0] != -1 && fds[0] != 0);
-            assert!(fds[1] != -1 && fds[1] != 0);
+    unsafe {
+        match libc::pipe(fds.as_mut_ptr(), 1024 as ::libc::c_uint,
+                         (libc::O_BINARY | libc::O_NOINHERIT) as c_int) {
+            0 => {
+                assert!(fds[0] != -1 && fds[0] != 0);
+                assert!(fds[1] != -1 && fds[1] != 0);
 
-            Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
+                Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1])))
+            }
+            _ => Err(io::Error::last_os_error()),
         }
-        _ => Err(io::Error::last_os_error()),
     }
 }
 
diff --git a/src/libstd/sys/windows/process2.rs b/src/libstd/sys/windows/process2.rs
index 7e832b6384d..74953921921 100644
--- a/src/libstd/sys/windows/process2.rs
+++ b/src/libstd/sys/windows/process2.rs
@@ -105,11 +105,18 @@ pub struct Process {
     handle: Handle,
 }
 
+pub enum Stdio {
+    Inherit,
+    Piped(AnonPipe),
+    None,
+}
+
 impl Process {
     #[allow(deprecated)]
     pub fn spawn(cfg: &Command,
-                 in_fd: Option<AnonPipe>, out_fd: Option<AnonPipe>, err_fd: Option<AnonPipe>)
-                 -> io::Result<Process>
+                 in_fd: Stdio,
+                 out_fd: Stdio,
+                 err_fd: Stdio) -> io::Result<Process>
     {
         use libc::types::os::arch::extra::{DWORD, HANDLE, STARTUPINFO};
         use libc::consts::os::extra::{
@@ -156,13 +163,16 @@ impl Process {
 
             let cur_proc = GetCurrentProcess();
 
-            // Similarly to unix, we don't actually leave holes for the stdio file
-            // descriptors, but rather open up /dev/null equivalents. These
-            // equivalents are drawn from libuv's windows process spawning.
-            let set_fd = |fd: &Option<AnonPipe>, slot: &mut HANDLE,
+            let set_fd = |fd: &Stdio, slot: &mut HANDLE,
                           is_stdin: bool| {
                 match *fd {
-                    None => {
+                    Stdio::Inherit => {}
+
+                    // Similarly to unix, we don't actually leave holes for the
+                    // stdio file descriptors, but rather open up /dev/null
+                    // equivalents. These equivalents are drawn from libuv's
+                    // windows process spawning.
+                    Stdio::None => {
                         let access = if is_stdin {
                             libc::FILE_GENERIC_READ
                         } else {
@@ -188,11 +198,8 @@ impl Process {
                             return Err(Error::last_os_error())
                         }
                     }
-                    Some(ref pipe) => {
+                    Stdio::Piped(ref pipe) => {
                         let orig = pipe.raw();
-                        if orig == INVALID_HANDLE_VALUE {
-                            return Err(Error::last_os_error())
-                        }
                         if DuplicateHandle(cur_proc, orig, cur_proc, slot,
                                            0, TRUE, DUPLICATE_SAME_ACCESS) == FALSE {
                             return Err(Error::last_os_error())
@@ -235,9 +242,15 @@ impl Process {
                 })
             });
 
-            assert!(CloseHandle(si.hStdInput) != 0);
-            assert!(CloseHandle(si.hStdOutput) != 0);
-            assert!(CloseHandle(si.hStdError) != 0);
+            if !in_fd.inherited() {
+                assert!(CloseHandle(si.hStdInput) != 0);
+            }
+            if !out_fd.inherited() {
+                assert!(CloseHandle(si.hStdOutput) != 0);
+            }
+            if !err_fd.inherited() {
+                assert!(CloseHandle(si.hStdError) != 0);
+            }
 
             match create_err {
                 Some(err) => return Err(err),
@@ -296,6 +309,12 @@ impl Process {
     }
 }
 
+impl Stdio {
+    fn inherited(&self) -> bool {
+        match *self { Stdio::Inherit => true, _ => false }
+    }
+}
+
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct ExitStatus(i32);