su: clean up groups initialization

This patch does not change any su/runuser behaviour, code changes:

* don't use huge groups[NGROUPS_MAX]; the array has 256k, but we need
  it only occasionally when -G/-g specified.

* the current code uses groups[0] for -g and the rest for -G, this patch adds
  'gid' to remember -g argument to avoid memmove()

* add function add_supp_group() to simplify su_main()

* add note about -G and -g relation to the man pages (undocumented now)

Signed-off-by: Karel Zak <kzak@redhat.com>

1 files changed, 38 insertions, 32 deletions

diff --git a/login-utils/su-common.c b/login-utils/su-common.c
index 2d4707330..fe6d0c8c3 100644
--- a/login-utils/su-common.c
+++ b/login-utils/su-common.c
@@ -575,7 +575,7 @@ modify_environment (const struct passwd *pw, const char *shell)
 /* Become the user and group(s) specified by PW.  */
 
 static void
-init_groups (const struct passwd *pw, gid_t *groups, int num_groups)
+init_groups (const struct passwd *pw, gid_t *groups, size_t num_groups)
 {
   int retval;
 
@@ -746,6 +746,28 @@ evaluate_uid(void)
   return (uid_t) 0 == ruid && ruid == euid ? 0 : 1;
 }
 
+static gid_t
+add_supp_group(const char *name, gid_t **groups, size_t *ngroups)
+{
+  struct group *gr;
+
+  if (*ngroups >= NGROUPS_MAX)
+    errx(EXIT_FAILURE,
+	P_("specifying more than %d supplemental group is not possible",
+	   "specifying more than %d supplemental groups is not possible",
+	     NGROUPS_MAX - 1), NGROUPS_MAX - 1);
+
+  gr = getgrnam(name);
+  if (!gr)
+    errx(EXIT_FAILURE, _("group %s does not exist"), name);
+
+  *groups = xrealloc(*groups, sizeof(gid_t) * (*ngroups + 1));
+  (*groups)[*ngroups] = gr->gr_gid;
+  (*ngroups)++;
+
+  return gr->gr_gid;
+}
+
 int
 su_main (int argc, char **argv, int mode)
 {
@@ -756,10 +778,12 @@ su_main (int argc, char **argv, int mode)
   char *shell = NULL;
   struct passwd *pw;
   struct passwd pw_copy;
-  struct group *gr;
-  gid_t groups[NGROUPS_MAX];
-  int num_supp_groups = 0;
-  int use_gid = 0;
+
+  gid_t *groups = NULL;
+  size_t ngroups = 0;
+  bool use_supp = false;
+  bool use_gid = false;
+  gid_t gid = 0;
 
   static const struct option longopts[] = {
     {"command", required_argument, NULL, 'c'},
@@ -804,25 +828,13 @@ su_main (int argc, char **argv, int mode)
 	  break;
 
 	case 'g':
-	  gr = getgrnam(optarg);
-	  if (!gr)
-	    errx(EXIT_FAILURE, _("group %s does not exist"), optarg);
-	  use_gid = 1;
-	  groups[0] = gr->gr_gid;
+	  use_gid = true;
+	  gid = add_supp_group(optarg, &groups, &ngroups);
 	  break;
 
 	case 'G':
-	  num_supp_groups++;
-	  if (num_supp_groups >= NGROUPS_MAX)
-	     errx(EXIT_FAILURE,
-		  P_("specifying more than %d supplemental group is not possible",
-		     "specifying more than %d supplemental groups is not possible",
-		     NGROUPS_MAX - 1),
-		  NGROUPS_MAX - 1);
-	  gr = getgrnam(optarg);
-	  if (!gr)
-	    errx(EXIT_FAILURE, _("group %s does not exist"), optarg);
-	  groups[num_supp_groups] = gr->gr_gid;
+	  use_supp = true;
+	  add_supp_group(optarg, &groups, &ngroups);
 	  break;
 
 	case 'l':
@@ -893,7 +905,7 @@ su_main (int argc, char **argv, int mode)
     break;
   }
 
-  if ((num_supp_groups || use_gid) && restricted)
+  if ((use_supp || use_gid) && restricted)
     errx(EXIT_FAILURE, _("only root can specify alternative groups"));
 
   logindefs_load_defaults = load_config;
@@ -919,16 +931,10 @@ su_main (int argc, char **argv, int mode)
 			  : DEFAULT_SHELL);
   endpwent ();
 
-  if (num_supp_groups && !use_gid)
-  {
-    pw->pw_gid = groups[1];
-    memmove (groups, groups + 1, sizeof(gid_t) * num_supp_groups);
-  }
-  else if (use_gid)
-  {
+  if (use_supp && !use_gid)
     pw->pw_gid = groups[0];
-    num_supp_groups++;
-  }
+  else if (use_gid)
+    pw->pw_gid = gid;
 
   authenticate (pw);
 
@@ -953,7 +959,7 @@ su_main (int argc, char **argv, int mode)
     shell = xstrdup (shell ? shell : pw->pw_shell);
   }
 
-  init_groups (pw, groups, num_supp_groups);
+  init_groups (pw, groups, ngroups);
 
   if (!simulate_login || command)
     suppress_pam_info = 1;		/* don't print PAM info messages */