From 5faf25b95cac4f5b5d893ca3791d00f95342a6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 8 Oct 2020 00:00:00 +0000 Subject: [PATCH 1/2] Check for non-zero return value from posix_spawn functions The cvt function compares the argument with -1 and when equal returns a new io::Error constructed from errno. It is used together posix_spawn_* functions. This is incorrect. Those functions do not set errno. Instead they return non-zero error code directly. Check for non-zero return code and use it to construct a new io::Error. --- .../std/src/sys/unix/process/process_unix.rs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index eb600d2465ca0..520e5e5d1c59a 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -339,6 +339,10 @@ impl Command { } } + fn cvt_nz(error: libc::c_int) -> io::Result<()> { + if error == 0 { Ok(()) } else { Err(io::Error::from_raw_os_error(error)) } + } + unsafe { let mut file_actions = PosixSpawnFileActions(MaybeUninit::uninit()); let mut attrs = PosixSpawnattr(MaybeUninit::uninit()); @@ -347,51 +351,51 @@ impl Command { libc::posix_spawn_file_actions_init(file_actions.0.as_mut_ptr()); if let Some(fd) = stdio.stdin.fd() { - cvt(libc::posix_spawn_file_actions_adddup2( + cvt_nz(libc::posix_spawn_file_actions_adddup2( file_actions.0.as_mut_ptr(), fd, libc::STDIN_FILENO, ))?; } if let Some(fd) = stdio.stdout.fd() { - cvt(libc::posix_spawn_file_actions_adddup2( + cvt_nz(libc::posix_spawn_file_actions_adddup2( file_actions.0.as_mut_ptr(), fd, libc::STDOUT_FILENO, ))?; } if let Some(fd) = stdio.stderr.fd() { - cvt(libc::posix_spawn_file_actions_adddup2( + cvt_nz(libc::posix_spawn_file_actions_adddup2( file_actions.0.as_mut_ptr(), fd, libc::STDERR_FILENO, ))?; } if let Some((f, cwd)) = addchdir { - cvt(f(file_actions.0.as_mut_ptr(), cwd.as_ptr()))?; + cvt_nz(f(file_actions.0.as_mut_ptr(), cwd.as_ptr()))?; } let mut set = MaybeUninit::::uninit(); cvt(sigemptyset(set.as_mut_ptr()))?; - cvt(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), set.as_ptr()))?; + cvt_nz(libc::posix_spawnattr_setsigmask(attrs.0.as_mut_ptr(), set.as_ptr()))?; cvt(sigaddset(set.as_mut_ptr(), libc::SIGPIPE))?; - cvt(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), set.as_ptr()))?; + cvt_nz(libc::posix_spawnattr_setsigdefault(attrs.0.as_mut_ptr(), set.as_ptr()))?; let flags = libc::POSIX_SPAWN_SETSIGDEF | libc::POSIX_SPAWN_SETSIGMASK; - cvt(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?; + cvt_nz(libc::posix_spawnattr_setflags(attrs.0.as_mut_ptr(), flags as _))?; // Make sure we synchronize access to the global `environ` resource let _env_lock = sys::os::env_lock(); let envp = envp.map(|c| c.as_ptr()).unwrap_or_else(|| *sys::os::environ() as *const _); - let ret = libc::posix_spawnp( + cvt_nz(libc::posix_spawnp( &mut p.pid, self.get_program_cstr().as_ptr(), file_actions.0.as_ptr(), attrs.0.as_ptr(), self.get_argv().as_ptr() as *const _, envp as *const _, - ); - if ret == 0 { Ok(Some(p)) } else { Err(io::Error::from_raw_os_error(ret)) } + ))?; + Ok(Some(p)) } } } From 6cd5506897d5aadb57455f5081ff8efd77f93b69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 8 Oct 2020 00:00:00 +0000 Subject: [PATCH 2/2] Check for errors returned from posix_spawn*_init functions The posix_spawnattr_init & posix_spawn_file_actions_init might fail, but their return code is not checked. Check for non-zero return code and destroy only succesfully initialized objects. --- .../std/src/sys/unix/process/process_unix.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 520e5e5d1c59a..5e55f97705db6 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -319,9 +319,9 @@ impl Command { let mut p = Process { pid: 0, status: None }; - struct PosixSpawnFileActions(MaybeUninit); + struct PosixSpawnFileActions<'a>(&'a mut MaybeUninit); - impl Drop for PosixSpawnFileActions { + impl Drop for PosixSpawnFileActions<'_> { fn drop(&mut self) { unsafe { libc::posix_spawn_file_actions_destroy(self.0.as_mut_ptr()); @@ -329,9 +329,9 @@ impl Command { } } - struct PosixSpawnattr(MaybeUninit); + struct PosixSpawnattr<'a>(&'a mut MaybeUninit); - impl Drop for PosixSpawnattr { + impl Drop for PosixSpawnattr<'_> { fn drop(&mut self) { unsafe { libc::posix_spawnattr_destroy(self.0.as_mut_ptr()); @@ -344,11 +344,13 @@ impl Command { } unsafe { - let mut file_actions = PosixSpawnFileActions(MaybeUninit::uninit()); - let mut attrs = PosixSpawnattr(MaybeUninit::uninit()); + let mut attrs = MaybeUninit::uninit(); + cvt_nz(libc::posix_spawnattr_init(attrs.as_mut_ptr()))?; + let attrs = PosixSpawnattr(&mut attrs); - libc::posix_spawnattr_init(attrs.0.as_mut_ptr()); - libc::posix_spawn_file_actions_init(file_actions.0.as_mut_ptr()); + let mut file_actions = MaybeUninit::uninit(); + cvt_nz(libc::posix_spawn_file_actions_init(file_actions.as_mut_ptr()))?; + let file_actions = PosixSpawnFileActions(&mut file_actions); if let Some(fd) = stdio.stdin.fd() { cvt_nz(libc::posix_spawn_file_actions_adddup2(