lib/, src/: Simplify allocation of buffer

getgroups(0, NULL) returns the number of groups, so that we can allocate
at once.  This might fail if there's a race and the number of users
grows while we're allocating, but if that happens, failing is probably a
good thing to do.

There was some comment saying it doesn't work on some systems, but
according to gnulib, that's only NeXTstep 3.2, which we don't support.

Link: <https://www.gnu.org/software/gnulib/manual/html_node/getgroups.html>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/lib/addgrps.c b/lib/addgrps.c
index 809b8e702a41da9b5060f8ae755bd95b2ec309dc..8a1667c1e507b19cf73edabadeb20254cb8c7bde 100644 (file)
--- a/lib/addgrps.c
+++ b/lib/addgrps.c
@@ -46,21 +46,17 @@ add_groups(const char *list)
        }
        strcpy (buf, list);
 
-       for (size_t i = 16; /* void */; i *= 2) {
-               grouplist = MALLOC(i, GETGROUPS_T);
-               if (NULL == grouplist) {
-                       return -1;
-               }
-               ngroups = getgroups (i, grouplist);
-               if (ngroups == -1 && errno != EINVAL) {
-                       free(grouplist);
-                       return -1;
-               }
-               if (i > (size_t)ngroups) {
-                       break;
-               }
-               free (grouplist);
-       }
+       ngroups = getgroups(0, NULL);
+       if (ngroups == -1)
+               return -1;
+
+       grouplist = MALLOC(ngroups, GETGROUPS_T);
+       if (grouplist == NULL)
+               return -1;
+
+       ngroups = getgroups(ngroups, grouplist);
+       if (ngroups == -1)
+               goto free_gids;
 
        added = false;
        p = buf;
@@ -103,6 +99,10 @@ add_groups(const char *list)
 
        free (grouplist);
        return 0;
+
+free_gids:
+       free(grouplist);
+       return -1;
 }
 #else                          /* !USE_PAM */
 extern int ISO_C_forbids_an_empty_translation_unit;

diff --git a/src/newgrp.c b/src/newgrp.c
index e5dc2290beb22452e10d3d00d9f48f8da0df0f75..823d2fe0a931aab0329e989bdef934506457336f 100644 (file)
--- a/src/newgrp.c
+++ b/src/newgrp.c
@@ -554,28 +554,28 @@ int main (int argc, char **argv)
         * nasty message but at least your real and effective group ids are
         * set.
         */
-       /* don't use getgroups(0, 0) - it doesn't work on some systems */
-       for (int i = 16; /* void */; i *= 2) {
-               grouplist = XMALLOC(i, GETGROUPS_T);
-               ngroups = getgroups (i, grouplist);
-               if (ngroups == -1 && errno != EINVAL) {
-                       perror("getgroups");
+
+       ngroups = getgroups(0, NULL);
+       if (ngroups == -1)
+               goto fail_gg;
+
+       grouplist = XMALLOC(ngroups, GETGROUPS_T);
+
+       ngroups = getgroups(ngroups, grouplist);
+       if (ngroups == -1) {
+fail_gg:
+               perror("getgroups");
 #ifdef WITH_AUDIT
-                       if (group) {
-                               SNPRINTF(audit_buf, "changing new-group=%s", group);
-                               audit_logger(AUDIT_CHGRP_ID, Prog,
-                                            audit_buf, NULL, getuid(), 0);
-                       } else {
-                               audit_logger(AUDIT_CHGRP_ID, Prog,
-                                            "changing", NULL, getuid(), 0);
-                       }
-#endif
-                       exit(EXIT_FAILURE);
-               }
-               if (i > (size_t)ngroups) {
-                       break;
+               if (group) {
+                       SNPRINTF(audit_buf, "changing new-group=%s", group);
+                       audit_logger(AUDIT_CHGRP_ID, Prog,
+                                    audit_buf, NULL, getuid(), 0);
+               } else {
+                       audit_logger(AUDIT_CHGRP_ID, Prog,
+                                    "changing", NULL, getuid(), 0);
                }
-               free (grouplist);
+#endif
+               exit(EXIT_FAILURE);
        }
 
        /*
@@ -685,6 +685,8 @@ int main (int argc, char **argv)
         * If the group doesn't fit, I'll complain loudly and skip this
         * part.
         */
+       grouplist = XREALLOC(grouplist, ngroups + 1, GETGROUPS_T);
+
        int i;
        for (i = 0; i < ngroups; i++) {
                if (gid == grouplist[i]) {