Merge pull request #987 from GuillaumeGomez/fix-group_retrieval

Handle errors correctly when getting group information on linux and freebsd

examples/simple.rs

@@ -122,7 +122,10 @@ fn print_help() {
         &mut io::stdout(),
         "frequency          : Displays CPU frequency"
     );
-    writeln!(&mut io::stdout(), "users              : Displays all users");
+    writeln!(
+        &mut io::stdout(),
+        "users              : Displays all users and their groups"
+    );
     writeln!(
         &mut io::stdout(),
         "system             : Displays system information (such as name, version and hostname)"
@@ -325,7 +328,12 @@ fn interpret_input(input: &str, sys: &mut System) -> bool {
         }
         "users" => {
             for user in sys.users() {
-                writeln!(&mut io::stdout(), "{:?}", user.name());
+                writeln!(
+                    &mut io::stdout(),
+                    "{:?} => {:?}",
+                    user.name(),
+                    user.groups()
+                );
             }
         }
         "boot_time" => {

src/apple/macos/process.rs

@@ -316,7 +316,7 @@ fn check_if_pid_is_alive(pid: Pid, check_if_alive: bool) -> bool {
             return true;
         }
         // `kill` failed but it might not be because the process is dead.
-        let errno = libc::__error();
+        let errno = crate::libc_errno();
         // If errno is equal to ESCHR, it means the process is dead.
         !errno.is_null() && *errno != libc::ESRCH
     }

src/apple/users.rs

@@ -6,47 +6,8 @@ use crate::{
 };
 
 use crate::sys::utils;
-use libc::{c_char, endpwent, getgrgid, getgrouplist, getpwent, gid_t, setpwent, strlen};
-
-fn get_user_groups(name: *const c_char, group_id: gid_t) -> Vec<String> {
-    let mut add = 0;
-
-    loop {
-        let mut nb_groups = 256 + add;
-        let mut groups = Vec::with_capacity(nb_groups as _);
-        unsafe {
-            if getgrouplist(name, group_id as _, groups.as_mut_ptr(), &mut nb_groups) == -1 {
-                add += 100;
-                continue;
-            }
-            groups.set_len(nb_groups as _);
-            return groups
-                .into_iter()
-                .filter_map(|g| {
-                    let errno = libc::__error();
-
-                    loop {
-                        // As mentioned in the man, we set `errno` to 0 to ensure that if a problem
-                        // occurs and errno is 0, then it means this group doesn't exist.
-                        if !errno.is_null() {
-                            *errno = 0;
-                        }
-
-                        let group = getgrgid(g as _);
-                        if group.is_null() {
-                            // The call was interrupted by a signal, retrying.
-                            if !errno.is_null() && *errno == libc::EINTR {
-                                continue;
-                            }
-                            return None;
-                        }
-                        return utils::cstr_to_rust((*group).gr_name);
-                    }
-                })
-                .collect();
-        }
-    }
-}
+use libc::{c_char, endpwent, getpwent, setpwent, strlen};
+use std::collections::HashMap;
 
 fn endswith(s1: *const c_char, s2: &[u8]) -> bool {
     if s1.is_null() {
@@ -67,7 +28,9 @@ fn users_list<F>(filter: F) -> Vec<User>
 where
     F: Fn(*const c_char, u32) -> bool,
 {
-    let mut users = Vec::new();
+    let mut users = HashMap::with_capacity(10);
+    let mut buffer = Vec::with_capacity(2048);
+    let mut groups = Vec::with_capacity(256);
 
     unsafe {
         setpwent();
@@ -85,24 +48,33 @@ where
                 // This is not a "real" or "local" user.
                 continue;
             }
-
-            let groups = get_user_groups((*pw).pw_name, (*pw).pw_gid);
-            let uid = (*pw).pw_uid;
-            let gid = (*pw).pw_gid;
             if let Some(name) = utils::cstr_to_rust((*pw).pw_name) {
-                users.push(User {
-                    uid: Uid(uid),
-                    gid: Gid(gid),
-                    name,
-                    groups,
-                });
+                if users.contains_key(&name) {
+                    continue;
+                }
+
+                let groups = crate::users::get_user_groups(
+                    (*pw).pw_name,
+                    (*pw).pw_gid,
+                    &mut groups,
+                    &mut buffer,
+                );
+                let uid = (*pw).pw_uid;
+                let gid = (*pw).pw_gid;
+                users.insert(name, (Uid(uid), Gid(gid), groups));
             }
         }
         endpwent();
     }
-    users.sort_unstable_by(|x, y| x.name.partial_cmp(&y.name).unwrap());
-    users.dedup_by(|a, b| a.name == b.name);
     users
+        .into_iter()
+        .map(|(name, (uid, gid, groups))| User {
+            uid,
+            gid,
+            name,
+            groups,
+        })
+        .collect()
 }
 
 pub(crate) fn get_users_list() -> Vec<User> {

src/lib.rs

@@ -25,10 +25,15 @@ cfg_if::cfg_if! {
     } else if #[cfg(any(target_os = "macos", target_os = "ios"))] {
         mod apple;
         use apple as sys;
+        pub(crate) mod users;
         mod network_helper_nix;
         use network_helper_nix as network_helper;
         mod network;
 
+        // This is needed because macos uses `int*` for `getgrouplist`...
+        pub(crate) type GroupId = libc::c_int;
+        pub(crate) use libc::__error as libc_errno;
+
         #[cfg(test)]
         pub(crate) const MIN_USERS: usize = 1;
     } else if #[cfg(windows)] {
@@ -48,6 +53,13 @@ cfg_if::cfg_if! {
         use network_helper_nix as network_helper;
         mod network;
 
+        // This is needed because macos uses `int*` for `getgrouplist`...
+        pub(crate) type GroupId = libc::gid_t;
+        #[cfg(target_os = "linux")]
+        pub(crate) use libc::__errno_location as libc_errno;
+        #[cfg(target_os = "android")]
+        pub(crate) use libc::__errno as libc_errno;
+
         #[cfg(test)]
         pub(crate) const MIN_USERS: usize = 1;
     } else if #[cfg(target_os = "freebsd")] {
@@ -58,6 +70,10 @@ cfg_if::cfg_if! {
         use network_helper_nix as network_helper;
         mod network;
 
+        // This is needed because macos uses `int*` for `getgrouplist`...
+        pub(crate) type GroupId = libc::gid_t;
+        pub(crate) use libc::__error as libc_errno;
+
         #[cfg(test)]
         pub(crate) const MIN_USERS: usize = 1;
     } else {

src/macros.rs

@@ -59,13 +59,33 @@ macro_rules! declare_signals {
 
 #[cfg(all(unix, not(feature = "unknown-ci")))]
 macro_rules! retry_eintr {
-    ($($t:tt)+) => {
+    (set_to_0 => $($t:tt)+) => {{
+        let errno = crate::libc_errno();
+        if !errno.is_null() {
+            *errno = 0;
+        }
+        retry_eintr!($($t)+)
+    }};
+    ($errno_value:ident => $($t:tt)+) => {{
+        loop {
+            let ret = $($t)+;
+            if ret < 0 {
+                let tmp = std::io::Error::last_os_error();
+                if tmp.kind() == std::io::ErrorKind::Interrupted {
+                    continue;
+                }
+                $errno_value = tmp.raw_os_error().unwrap_or(0);
+            }
+            break ret;
+        }
+    }};
+    ($($t:tt)+) => {{
         loop {
             let ret = $($t)+;
             if ret < 0 && std::io::Error::last_os_error().kind() == std::io::ErrorKind::Interrupted {
                 continue;
             }
             break ret;
         }
-    }
+    }};
 }

src/users.rs

@@ -9,10 +9,83 @@ use libc::{getgrgid_r, getgrouplist};
 use std::fs::File;
 use std::io::Read;
 
-pub fn get_users_list() -> Vec<User> {
+pub(crate) unsafe fn get_group_name(
+    id: libc::gid_t,
+    buffer: &mut Vec<libc::c_char>,
+) -> Option<String> {
+    let mut g = std::mem::MaybeUninit::<libc::group>::uninit();
+    let mut tmp_ptr = std::ptr::null_mut();
+    let mut last_errno = 0;
+    loop {
+        if retry_eintr!(set_to_0 => last_errno => getgrgid_r(
+            id as _,
+            g.as_mut_ptr() as _,
+            buffer.as_mut_ptr(),
+            buffer.capacity() as _,
+            &mut tmp_ptr as _
+        )) != 0
+        {
+            // If there was not enough memory, we give it more.
+            if last_errno == libc::ERANGE as _ {
+                buffer.reserve(2048);
+                continue;
+            }
+            return None;
+        }
+        break;
+    }
+    let g = g.assume_init();
+    let mut group_name = Vec::new();
+    let c_group_name = g.gr_name;
+    let mut x = 0;
+    loop {
+        let c = *c_group_name.offset(x);
+        if c == 0 {
+            break;
+        }
+        group_name.push(c as u8);
+        x += 1;
+    }
+    String::from_utf8(group_name).ok()
+}
+
+pub(crate) unsafe fn get_user_groups(
+    name: *const libc::c_char,
+    group_id: libc::gid_t,
+    groups: &mut Vec<crate::GroupId>,
+    buffer: &mut Vec<libc::c_char>,
+) -> Vec<String> {
+    loop {
+        let mut nb_groups = groups.capacity();
+        if getgrouplist(
+            name,
+            group_id as _,
+            groups.as_mut_ptr(),
+            &mut nb_groups as *mut _ as *mut _,
+        ) == -1
+        {
+            groups.reserve(256);
+            continue;
+        }
+        groups.set_len(nb_groups as _);
+        return groups
+            .iter()
+            .filter_map(|group_id| crate::users::get_group_name(*group_id as _, buffer))
+            .collect();
+    }
+}
+
+// Not used by mac.
+#[allow(unused)]
+pub(crate) fn get_users_list() -> Vec<User> {
+    #[inline]
+    fn parse_id(id: &str) -> Option<u32> {
+        id.parse::<u32>().ok()
+    }
+
     let mut s = String::new();
-    let mut ngroups = 100;
-    let mut groups = vec![0; ngroups as usize];
+    let mut buffer = Vec::with_capacity(2048);
+    let mut groups = Vec::with_capacity(256);
 
     let _ = File::open("/etc/passwd").and_then(|mut f| f.read_to_string(&mut s));
     s.lines()
@@ -25,74 +98,24 @@ pub fn get_users_list() -> Vec<User> {
                     if let Some(group_id) = parts.next().and_then(parse_id) {
                         let mut c_user = username.as_bytes().to_vec();
                         c_user.push(0);
-                        loop {
-                            let mut current = ngroups;
-
-                            unsafe {
-                                if getgrouplist(
+                        // Let's get all the group names!
+                        return Some(User {
+                            uid: Uid(uid),
+                            gid: Gid(group_id),
+                            name: username.to_owned(),
+                            groups: unsafe {
+                                get_user_groups(
                                     c_user.as_ptr() as *const _,
                                     group_id,
-                                    groups.as_mut_ptr(),
-                                    &mut current,
-                                ) == -1
-                                {
-                                    if current > ngroups {
-                                        groups.resize(current as _, 0);
-                                        ngroups = current;
-                                        continue;
-                                    }
-                                    // It really failed, let's move on...
-                                    return None;
-                                }
-                                // Let's get all the group names!
-                                return Some(User {
-                                    uid: Uid(uid),
-                                    gid: Gid(group_id),
-                                    name: username.to_owned(),
-                                    groups: groups[..current as usize]
-                                        .iter()
-                                        .filter_map(|id| {
-                                            let mut g =
-                                                std::mem::MaybeUninit::<libc::group>::uninit();
-                                            let mut tmp_ptr = std::ptr::null_mut();
-                                            let mut buf = Vec::with_capacity(2048);
-                                            if retry_eintr!(getgrgid_r(
-                                                *id as _,
-                                                g.as_mut_ptr() as _,
-                                                buf.as_mut_ptr(),
-                                                buf.capacity() as _,
-                                                &mut tmp_ptr as _
-                                            )) != 0
-                                            {
-                                                return None;
-                                            }
-                                            let g = g.assume_init();
-                                            let mut group_name = Vec::new();
-                                            let c_group_name = g.gr_name;
-                                            let mut x = 0;
-                                            loop {
-                                                let c = *c_group_name.offset(x);
-                                                if c == 0 {
-                                                    break;
-                                                }
-                                                group_name.push(c as u8);
-                                                x += 1;
-                                            }
-                                            String::from_utf8(group_name).ok()
-                                        })
-                                        .collect(),
-                                });
-                            }
-                        }
+                                    &mut groups,
+                                    &mut buffer,
+                                )
+                            },
+                        });
                     }
                 }
             }
             None
         })
         .collect()
 }
-
-#[inline]
-fn parse_id(id: &str) -> Option<u32> {
-    id.parse::<u32>().ok()
-}