diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index d37a59465..b68da6eb7 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -182,6 +182,7 @@ getgrgid getgrnam getgrouplist getgroups +getpwent getpwnam getpwuid getuid diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index f24c4ec89..7b0c94810 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -183,7 +183,7 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { let uid = if !user.is_empty() { Some(match Passwd::locate(user) { - Ok(u) => u.uid(), // We have been able to get the uid + Ok(u) => u.uid, // We have been able to get the uid Err(_) => // we have NOT been able to find the uid // but we could be in the case where we have user.group @@ -208,7 +208,7 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { Some( Group::locate(group) .map_err(|_| USimpleError::new(1, format!("invalid group: {}", spec.quote())))? - .gid(), + .gid, ) } else { None diff --git a/src/uu/id/src/id.rs b/src/uu/id/src/id.rs index 1229b577e..efe9a5d4e 100644 --- a/src/uu/id/src/id.rs +++ b/src/uu/id/src/id.rs @@ -245,7 +245,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // GNU's `id` does not support the flags: -p/-P/-A. if matches.is_present(options::OPT_PASSWORD) { // BSD's `id` ignores all but the first specified user - pline(possible_pw.map(|v| v.uid())); + pline(possible_pw.as_ref().map(|v| v.uid)); return Ok(()); }; if matches.is_present(options::OPT_HUMAN_READABLE) { @@ -259,7 +259,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { return Ok(()); } - let (uid, gid) = possible_pw.map(|p| (p.uid(), p.gid())).unwrap_or(( + let (uid, gid) = possible_pw.as_ref().map(|p| (p.uid, p.gid)).unwrap_or(( if state.rflag { getuid() } else { geteuid() }, if state.rflag { getgid() } else { getegid() }, )); @@ -302,7 +302,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let groups = entries::get_groups_gnu(Some(gid)).unwrap(); let groups = if state.user_specified { - possible_pw.map(|p| p.belongs_to()).unwrap() + possible_pw.as_ref().map(|p| p.belongs_to()).unwrap() } else { groups.clone() }; @@ -453,7 +453,7 @@ pub fn uu_app() -> App<'static, 'static> { fn pretty(possible_pw: Option) { if let Some(p) = possible_pw { - print!("uid\t{}\ngroups\t", p.name()); + print!("uid\t{}\ngroups\t", p.name); println!( "{}", p.belongs_to() @@ -466,10 +466,10 @@ fn pretty(possible_pw: Option) { let login = cstr2cow!(getlogin() as *const _); let rid = getuid(); if let Ok(p) = Passwd::locate(rid) { - if login == p.name() { + if login == p.name { println!("login\t{}", login); } - println!("uid\t{}", p.name()); + println!("uid\t{}", p.name); } else { println!("uid\t{}", rid); } @@ -477,7 +477,7 @@ fn pretty(possible_pw: Option) { let eid = getegid(); if eid == rid { if let Ok(p) = Passwd::locate(eid) { - println!("euid\t{}", p.name()); + println!("euid\t{}", p.name); } else { println!("euid\t{}", eid); } @@ -486,7 +486,7 @@ fn pretty(possible_pw: Option) { let rid = getgid(); if rid != eid { if let Ok(g) = Group::locate(rid) { - println!("euid\t{}", g.name()); + println!("euid\t{}", g.name); } else { println!("euid\t{}", rid); } @@ -511,16 +511,16 @@ fn pline(possible_uid: Option) { println!( "{}:{}:{}:{}:{}:{}:{}:{}:{}:{}", - pw.name(), - pw.user_passwd(), - pw.uid(), - pw.gid(), - pw.user_access_class(), - pw.passwd_change_time(), - pw.expiration(), - pw.user_info(), - pw.user_dir(), - pw.user_shell() + pw.name, + pw.user_passwd, + pw.uid, + pw.gid, + pw.user_access_class, + pw.passwd_change_time, + pw.expiration, + pw.user_info, + pw.user_dir, + pw.user_shell ); } @@ -531,13 +531,7 @@ fn pline(possible_uid: Option) { println!( "{}:{}:{}:{}:{}:{}:{}", - pw.name(), - pw.user_passwd(), - pw.uid(), - pw.gid(), - pw.user_info(), - pw.user_dir(), - pw.user_shell() + pw.name, pw.user_passwd, pw.uid, pw.gid, pw.user_info, pw.user_dir, pw.user_shell ); } diff --git a/src/uu/pinky/src/pinky.rs b/src/uu/pinky/src/pinky.rs index 4aa27affa..487ceaf0a 100644 --- a/src/uu/pinky/src/pinky.rs +++ b/src/uu/pinky/src/pinky.rs @@ -267,11 +267,11 @@ impl Pinky { if self.include_fullname { if let Ok(pw) = Passwd::locate(ut.user().as_ref()) { - let mut gecos = pw.user_info().into_owned(); + let mut gecos = pw.user_info; if let Some(n) = gecos.find(',') { gecos.truncate(n + 1); } - print!(" {:<19.19}", gecos.replace("&", &pw.name().capitalize())); + print!(" {:<19.19}", gecos.replace("&", &pw.name.capitalize())); } else { print!(" {:19}", " ???"); } @@ -333,13 +333,13 @@ impl Pinky { for u in &self.names { print!("Login name: {:<28}In real life: ", u); if let Ok(pw) = Passwd::locate(u.as_str()) { - println!(" {}", pw.user_info().replace("&", &pw.name().capitalize())); + println!(" {}", pw.user_info.replace("&", &pw.name.capitalize())); if self.include_home_and_shell { - print!("Directory: {:<29}", pw.user_dir()); - println!("Shell: {}", pw.user_shell()); + print!("Directory: {:<29}", pw.user_dir); + println!("Shell: {}", pw.user_shell); } if self.include_project { - let mut p = PathBuf::from(pw.user_dir().as_ref()); + let mut p = PathBuf::from(&pw.user_dir); p.push(".project"); if let Ok(f) = File::open(p) { print!("Project: "); @@ -347,7 +347,7 @@ impl Pinky { } } if self.include_plan { - let mut p = PathBuf::from(pw.user_dir().as_ref()); + let mut p = PathBuf::from(&pw.user_dir); p.push(".plan"); if let Ok(f) = File::open(p) { println!("Plan:"); diff --git a/src/uucore/src/lib/features/entries.rs b/src/uucore/src/lib/features/entries.rs index f139d6871..df3ab7b06 100644 --- a/src/uucore/src/lib/features/entries.rs +++ b/src/uucore/src/lib/features/entries.rs @@ -41,12 +41,15 @@ use libc::{c_char, c_int, gid_t, uid_t}; use libc::{getgrgid, getgrnam, getgroups}; use libc::{getpwnam, getpwuid, group, passwd}; -use std::borrow::Cow; +use std::convert::TryInto; use std::ffi::{CStr, CString}; use std::io::Error as IOError; use std::io::ErrorKind; use std::io::Result as IOResult; use std::ptr; +use std::sync::Mutex; + +use once_cell::sync::Lazy; extern "C" { /// From: https://man7.org/linux/man-pages/man3/getgrouplist.3.html @@ -69,19 +72,31 @@ extern "C" { /// > to be used in a further call to getgroups(). #[cfg(not(target_os = "redox"))] pub fn get_groups() -> IOResult> { - let ngroups = unsafe { getgroups(0, ptr::null_mut()) }; - if ngroups == -1 { - return Err(IOError::last_os_error()); - } - let mut groups = Vec::with_capacity(ngroups as usize); - let ngroups = unsafe { getgroups(ngroups, groups.as_mut_ptr()) }; - if ngroups == -1 { - Err(IOError::last_os_error()) - } else { - unsafe { - groups.set_len(ngroups as usize); + let mut groups = Vec::new(); + loop { + let ngroups = match unsafe { getgroups(0, ptr::null_mut()) } { + -1 => return Err(IOError::last_os_error()), + // Not just optimization; 0 would mess up the next call + 0 => return Ok(Vec::new()), + n => n, + }; + + // This is a small buffer, so we can afford to zero-initialize it and + // use safe Vec operations + groups.resize(ngroups.try_into().unwrap(), 0); + let res = unsafe { getgroups(ngroups, groups.as_mut_ptr()) }; + if res == -1 { + let err = IOError::last_os_error(); + if err.raw_os_error() == Some(libc::EINVAL) { + // Number of groups changed, retry + continue; + } else { + return Err(err); + } + } else { + groups.truncate(ngroups.try_into().unwrap()); + return Ok(groups); } - Ok(groups) } } @@ -124,77 +139,57 @@ fn sort_groups(mut groups: Vec, egid: gid_t) -> Vec { groups } -#[derive(Copy, Clone)] +#[derive(Clone, Debug)] pub struct Passwd { - inner: passwd, + /// AKA passwd.pw_name + pub name: String, + /// AKA passwd.pw_uid + pub uid: uid_t, + /// AKA passwd.pw_gid + pub gid: gid_t, + /// AKA passwd.pw_gecos + pub user_info: String, + /// AKA passwd.pw_shell + pub user_shell: String, + /// AKA passwd.pw_dir + pub user_dir: String, + /// AKA passwd.pw_passwd + pub user_passwd: String, + /// AKA passwd.pw_class + #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] + pub user_access_class: String, + /// AKA passwd.pw_change + #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] + pub passwd_change_time: time_t, + /// AKA passwd.pw_expire + #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] + pub expiration: time_t, } -macro_rules! cstr2cow { - ($v:expr) => { - unsafe { CStr::from_ptr($v).to_string_lossy() } - }; +/// SAFETY: ptr must point to a valid C string. +unsafe fn cstr2string(ptr: *const c_char) -> String { + CStr::from_ptr(ptr).to_string_lossy().into_owned() } impl Passwd { - /// AKA passwd.pw_name - pub fn name(&self) -> Cow { - cstr2cow!(self.inner.pw_name) - } - - /// AKA passwd.pw_uid - pub fn uid(&self) -> uid_t { - self.inner.pw_uid - } - - /// AKA passwd.pw_gid - pub fn gid(&self) -> gid_t { - self.inner.pw_gid - } - - /// AKA passwd.pw_gecos - pub fn user_info(&self) -> Cow { - cstr2cow!(self.inner.pw_gecos) - } - - /// AKA passwd.pw_shell - pub fn user_shell(&self) -> Cow { - cstr2cow!(self.inner.pw_shell) - } - - /// AKA passwd.pw_dir - pub fn user_dir(&self) -> Cow { - cstr2cow!(self.inner.pw_dir) - } - - /// AKA passwd.pw_passwd - pub fn user_passwd(&self) -> Cow { - cstr2cow!(self.inner.pw_passwd) - } - - /// AKA passwd.pw_class - #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] - pub fn user_access_class(&self) -> Cow { - cstr2cow!(self.inner.pw_class) - } - - /// AKA passwd.pw_change - #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] - pub fn passwd_change_time(&self) -> time_t { - self.inner.pw_change - } - - /// AKA passwd.pw_expire - #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] - pub fn expiration(&self) -> time_t { - self.inner.pw_expire - } - - pub fn as_inner(&self) -> &passwd { - &self.inner - } - - pub fn into_inner(self) -> passwd { - self.inner + /// SAFETY: All the pointed-to strings must be valid and not change while + /// the function runs. That means PW_LOCK must be held. + unsafe fn from_raw(raw: passwd) -> Self { + Passwd { + name: cstr2string(raw.pw_name), + uid: raw.pw_uid, + gid: raw.pw_gid, + user_info: cstr2string(raw.pw_gecos), + user_shell: cstr2string(raw.pw_shell), + user_dir: cstr2string(raw.pw_dir), + user_passwd: cstr2string(raw.pw_passwd), + #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] + user_access_class: cstr2string(raw.pw_class), + #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] + passwd_change_time: raw.pw_change, + #[cfg(any(target_os = "freebsd", target_vendor = "apple"))] + expiration: raw.pw_expire, + } } /// This is a wrapper function for `libc::getgrouplist`. @@ -214,49 +209,44 @@ impl Passwd { pub fn belongs_to(&self) -> Vec { let mut ngroups: c_int = 8; let mut ngroups_old: c_int; - let mut groups = Vec::with_capacity(ngroups as usize); - let gid = self.inner.pw_gid; - let name = self.inner.pw_name; + let mut groups = vec![0; ngroups.try_into().unwrap()]; + let name = CString::new(self.name.clone()).unwrap(); loop { ngroups_old = ngroups; - if unsafe { getgrouplist(name, gid, groups.as_mut_ptr(), &mut ngroups) } == -1 { + if unsafe { getgrouplist(name.as_ptr(), self.gid, groups.as_mut_ptr(), &mut ngroups) } + == -1 + { if ngroups == ngroups_old { ngroups *= 2; } - groups.resize(ngroups as usize, 0); + groups.resize(ngroups.try_into().unwrap(), 0); } else { break; } } - unsafe { - groups.set_len(ngroups as usize); - } - groups.truncate(ngroups as usize); + let ngroups = ngroups.try_into().unwrap(); + assert!(ngroups <= groups.len()); + groups.truncate(ngroups); groups } } +#[derive(Clone, Debug)] pub struct Group { - inner: group, + /// AKA group.gr_name + pub name: String, + /// AKA group.gr_gid + pub gid: gid_t, } impl Group { - /// AKA group.gr_name - pub fn name(&self) -> Cow { - cstr2cow!(self.inner.gr_name) - } - - /// AKA group.gr_gid - pub fn gid(&self) -> gid_t { - self.inner.gr_gid - } - - pub fn as_inner(&self) -> &group { - &self.inner - } - - pub fn into_inner(self) -> group { - self.inner + /// SAFETY: gr_name must be valid and not change while + /// the function runs. That means PW_LOCK must be held. + unsafe fn from_raw(raw: group) -> Self { + Group { + name: cstr2string(raw.gr_name), + gid: raw.gr_gid, + } } } @@ -267,17 +257,32 @@ pub trait Locate { Self: ::std::marker::Sized; } +// These functions are not thread-safe: +// > The return value may point to a static area, and may be +// > overwritten by subsequent calls to getpwent(3), getpwnam(), +// > or getpwuid(). +// This applies not just to the struct but also the strings it points +// to, so we must copy all the data we want before releasing the lock. +// (Technically we must also ensure that the raw functions aren't being called +// anywhere else in the program.) +static PW_LOCK: Lazy> = Lazy::new(|| Mutex::new(())); + macro_rules! f { ($fnam:ident, $fid:ident, $t:ident, $st:ident) => { impl Locate<$t> for $st { fn locate(k: $t) -> IOResult { + let _guard = PW_LOCK.lock(); + // SAFETY: We're holding PW_LOCK. unsafe { let data = $fid(k); if !data.is_null() { - Ok($st { - inner: ptr::read(data as *const _), - }) + Ok($st::from_raw(ptr::read(data as *const _))) } else { + // FIXME: Resource limits, signals and I/O failure may + // cause this too. See getpwnam(3). + // errno must be set to zero before the call. We can + // use libc::__errno_location() on some platforms. + // The same applies for the two cases below. Err(IOError::new( ErrorKind::NotFound, format!("No such id: {}", k), @@ -289,25 +294,26 @@ macro_rules! f { impl<'a> Locate<&'a str> for $st { fn locate(k: &'a str) -> IOResult { + let _guard = PW_LOCK.lock(); if let Ok(id) = k.parse::<$t>() { - let data = unsafe { $fid(id) }; - if !data.is_null() { - Ok($st { - inner: unsafe { ptr::read(data as *const _) }, - }) - } else { - Err(IOError::new( - ErrorKind::NotFound, - format!("No such id: {}", id), - )) + // SAFETY: We're holding PW_LOCK. + unsafe { + let data = $fid(id); + if !data.is_null() { + Ok($st::from_raw(ptr::read(data as *const _))) + } else { + Err(IOError::new( + ErrorKind::NotFound, + format!("No such id: {}", id), + )) + } } } else { + // SAFETY: We're holding PW_LOCK. unsafe { let data = $fnam(CString::new(k).unwrap().as_ptr()); if !data.is_null() { - Ok($st { - inner: ptr::read(data as *const _), - }) + Ok($st::from_raw(ptr::read(data as *const _))) } else { Err(IOError::new( ErrorKind::NotFound, @@ -327,24 +333,24 @@ f!(getgrnam, getgrgid, gid_t, Group); #[inline] pub fn uid2usr(id: uid_t) -> IOResult { - Passwd::locate(id).map(|p| p.name().into_owned()) + Passwd::locate(id).map(|p| p.name) } #[cfg(not(target_os = "redox"))] #[inline] pub fn gid2grp(id: gid_t) -> IOResult { - Group::locate(id).map(|p| p.name().into_owned()) + Group::locate(id).map(|p| p.name) } #[inline] pub fn usr2uid(name: &str) -> IOResult { - Passwd::locate(name).map(|p| p.uid()) + Passwd::locate(name).map(|p| p.uid) } #[cfg(not(target_os = "redox"))] #[inline] pub fn grp2gid(name: &str) -> IOResult { - Group::locate(name).map(|p| p.gid()) + Group::locate(name).map(|p| p.gid) } #[cfg(test)] diff --git a/tests/by-util/test_pinky.rs b/tests/by-util/test_pinky.rs index 5394dfde9..17cec1b4b 100644 --- a/tests/by-util/test_pinky.rs +++ b/tests/by-util/test_pinky.rs @@ -24,14 +24,11 @@ fn test_capitalize() { fn test_long_format() { let login = "root"; let pw: Passwd = Passwd::locate(login).unwrap(); - let real_name = pw.user_info().replace("&", &pw.name().capitalize()); + let real_name = pw.user_info.replace("&", &pw.name.capitalize()); let ts = TestScenario::new(util_name!()); ts.ucmd().arg("-l").arg(login).succeeds().stdout_is(format!( "Login name: {:<28}In real life: {}\nDirectory: {:<29}Shell: {}\n\n", - login, - real_name, - pw.user_dir(), - pw.user_shell() + login, real_name, pw.user_dir, pw.user_shell )); ts.ucmd()