From 9ebc63335ddecc605ffc56c03cddfca1eab8f273 Mon Sep 17 00:00:00 2001 From: Ta Thanh Dinh Date: Fri, 6 Sep 2019 03:08:46 +0200 Subject: [PATCH 1/2] fix #1093 --- CHANGELOG.md | 3 +++ src/unistd.rs | 23 +++++++++++------------ test/test_unistd.rs | 18 +++++++++--------- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af2069a0cb..7192d4b38a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). `self` by value. ([#1107](https://github.com/nix-rust/nix/pull/1107)) +- Type `&CString` for parameters of `exec(v|ve|vp|vpe|veat)` are changed to `&CStr`. + ([#1121](https://github.com/nix-rust/nix/pull/1121)) + ### Fixed - Fix length of abstract socket addresses ([#1120](https://github.com/nix-rust/nix/pull/1120)) diff --git a/src/unistd.rs b/src/unistd.rs index ea22f38ea0..2351509df9 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -7,7 +7,7 @@ use fcntl::FcntlArg::F_SETFD; use libc::{self, c_char, c_void, c_int, c_long, c_uint, size_t, pid_t, off_t, uid_t, gid_t, mode_t, PATH_MAX}; use std::{fmt, mem, ptr}; -use std::ffi::{CString, CStr, OsString, OsStr}; +use std::ffi::{CStr, OsString, OsStr}; use std::os::unix::ffi::{OsStringExt, OsStrExt}; use std::os::unix::io::RawFd; use std::path::PathBuf; @@ -675,10 +675,9 @@ pub fn fchownat( Errno::result(res).map(drop) } -fn to_exec_array(args: &[CString]) -> Vec<*const c_char> { - let mut args_p: Vec<*const c_char> = args.iter().map(|s| s.as_ptr()).collect(); - args_p.push(ptr::null()); - args_p +fn to_exec_array(args: &[&CStr]) -> Vec<*const c_char> { + use std::iter::once; + args.iter().map(|s| s.as_ptr()).chain(once(ptr::null())).collect() } /// Replace the current process image with a new one (see @@ -688,7 +687,7 @@ fn to_exec_array(args: &[CString]) -> Vec<*const c_char> { /// performs the same action but does not allow for customization of the /// environment for the new process. #[inline] -pub fn execv(path: &CString, argv: &[CString]) -> Result { +pub fn execv(path: &CStr, argv: &[&CStr]) -> Result { let args_p = to_exec_array(argv); unsafe { @@ -712,7 +711,7 @@ pub fn execv(path: &CString, argv: &[CString]) -> Result { /// in the `args` list is an argument to the new process. Each element in the /// `env` list should be a string in the form "key=value". #[inline] -pub fn execve(path: &CString, args: &[CString], env: &[CString]) -> Result { +pub fn execve(path: &CStr, args: &[&CStr], env: &[&CStr]) -> Result { let args_p = to_exec_array(args); let env_p = to_exec_array(env); @@ -733,7 +732,7 @@ pub fn execve(path: &CString, args: &[CString], env: &[CString]) -> Result /// would not work if "bash" was specified for the path argument, but `execvp` /// would assuming that a bash executable was on the system `PATH`. #[inline] -pub fn execvp(filename: &CString, args: &[CString]) -> Result { +pub fn execvp(filename: &CStr, args: &[&CStr]) -> Result { let args_p = to_exec_array(args); unsafe { @@ -753,7 +752,7 @@ pub fn execvp(filename: &CString, args: &[CString]) -> Result { #[cfg(any(target_os = "haiku", target_os = "linux", target_os = "openbsd"))] -pub fn execvpe(filename: &CString, args: &[CString], env: &[CString]) -> Result { +pub fn execvpe(filename: &CStr, args: &[&CStr], env: &[&CStr]) -> Result { let args_p = to_exec_array(args); let env_p = to_exec_array(env); @@ -781,7 +780,7 @@ pub fn execvpe(filename: &CString, args: &[CString], env: &[CString]) -> Result< target_os = "linux", target_os = "freebsd"))] #[inline] -pub fn fexecve(fd: RawFd, args: &[CString], env: &[CString]) -> Result { +pub fn fexecve(fd: RawFd, args: &[&CStr], env: &[&CStr]) -> Result { let args_p = to_exec_array(args); let env_p = to_exec_array(env); @@ -804,8 +803,8 @@ pub fn fexecve(fd: RawFd, args: &[CString], env: &[CString]) -> Result { /// is referenced as a file descriptor to the base directory plus a path. #[cfg(any(target_os = "android", target_os = "linux"))] #[inline] -pub fn execveat(dirfd: RawFd, pathname: &CString, args: &[CString], - env: &[CString], flags: super::fcntl::AtFlags) -> Result { +pub fn execveat(dirfd: RawFd, pathname: &CStr, args: &[&CStr], + env: &[&CStr], flags: super::fcntl::AtFlags) -> Result { let args_p = to_exec_array(args); let env_p = to_exec_array(env); diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 544c2b1345..9e45ceeb89 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -204,13 +204,13 @@ macro_rules! execve_test_factory( dup2(writer, 1).unwrap(); let r = $syscall( $exe, - $(&CString::new($pathname).unwrap(), )* - &[CString::new(b"".as_ref()).unwrap(), - CString::new(b"-c".as_ref()).unwrap(), + $(CString::new($pathname).unwrap().as_c_str(), )* + &[CString::new(b"".as_ref()).unwrap().as_c_str(), + CString::new(b"-c".as_ref()).unwrap().as_c_str(), CString::new(b"echo nix!!! && echo foo=$foo && echo baz=$baz" - .as_ref()).unwrap()], - &[CString::new(b"foo=bar".as_ref()).unwrap(), - CString::new(b"baz=quux".as_ref()).unwrap()] + .as_ref()).unwrap().as_c_str()], + &[CString::new(b"foo=bar".as_ref()).unwrap().as_c_str(), + CString::new(b"baz=quux".as_ref()).unwrap().as_c_str()] $(, $flags)*); let _ = std::io::stderr() .write_all(format!("{:?}", r).as_bytes()); @@ -238,18 +238,18 @@ macro_rules! execve_test_factory( cfg_if!{ if #[cfg(target_os = "android")] { - execve_test_factory!(test_execve, execve, &CString::new("/system/bin/sh").unwrap()); + execve_test_factory!(test_execve, execve, CString::new("/system/bin/sh").unwrap().as_c_str()); execve_test_factory!(test_fexecve, fexecve, File::open("/system/bin/sh").unwrap().into_raw_fd()); } else if #[cfg(any(target_os = "freebsd", target_os = "linux"))] { - execve_test_factory!(test_execve, execve, &CString::new("/bin/sh").unwrap()); + execve_test_factory!(test_execve, execve, CString::new("/bin/sh").unwrap().as_c_str()); execve_test_factory!(test_fexecve, fexecve, File::open("/bin/sh").unwrap().into_raw_fd()); } else if #[cfg(any(target_os = "dragonfly", target_os = "ios", target_os = "macos", target_os = "netbsd", target_os = "openbsd"))] { - execve_test_factory!(test_execve, execve, &CString::new("/bin/sh").unwrap()); + execve_test_factory!(test_execve, execve, CString::new("/bin/sh").unwrap().as_c_str()); // No fexecve() on DragonFly, ios, macos, NetBSD, OpenBSD. // // Note for NetBSD and OpenBSD: although rust-lang/libc includes it From e4814dd8569832e33dd8d2e462e2351b58cf1a2f Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 25 Sep 2019 17:32:11 -0300 Subject: [PATCH 2/2] unistd: getgrouplist: Rework code to use `reserve_double_buffer_size` The buffer resize logic can be simplified reusing the `reserve_double_buffer_size` method. Signed-off-by: Otavio Salvador --- src/unistd.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index dfe91e3291..fcbe1939b4 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -1486,19 +1486,8 @@ pub fn getgrouplist(user: &CStr, group: Gid) -> Result> { // BSD systems will still fill the groups buffer with as many // groups as possible, but Linux manpages do not mention this // behavior. - - let cap = groups.capacity(); - if cap >= ngroups_max as usize { - // We already have the largest capacity we can, give up - return Err(Error::invalid_argument()); - } - - // Reserve space for at least ngroups - groups.reserve(ngroups as usize); - - // Even if the buffer gets resized to bigger than ngroups_max, - // don't ever ask for more than ngroups_max groups - ngroups = min(ngroups_max, groups.capacity() as c_int); + reserve_double_buffer_size(&mut groups, ngroups_max as usize) + .or_else(|_| Err(Error::invalid_argument()))?; } } }