git: babab49eee94 - main - chroot: don't setgroups() without -G having been specified

The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=babab49eee9472f628d774996de13d13d296c8c0

commit babab49eee9472f628d774996de13d13d296c8c0
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2025-08-12 12:14:38 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2025-08-12 12:30:23 +0000

    chroot: don't setgroups() without -G having been specified
    
    We previously would not have setgroups() at all, but now we would drop
    our supplementary groups every time.  This broke chroot -n, probably
    among other things.  We need tests here, but lets unbreak things first.
    
    A future change may try to setgroups(2) when -u is specified in addition
    to -G, so predicate the call on gidlist and don't populate that without
    a grouplist.
    
    PR:             288751
    Fixes:  48fd05999b0f ("chroot: don't clobber the egid [...]")
---
 usr.sbin/chroot/chroot.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c
index d9fb29474d87..7ec5a00b50f0 100644
--- a/usr.sbin/chroot/chroot.c
+++ b/usr.sbin/chroot/chroot.c
@@ -103,7 +103,9 @@ main(int argc, char *argv[])
 
 	gid = 0;
 	uid = 0;
+	gids = 0;
 	user = group = grouplist = NULL;
+	gidlist = NULL;
 	nonprivileged = false;
 	while ((ch = getopt(argc, argv, "G:g:u:n")) != -1) {
 		switch(ch) {
@@ -119,6 +121,11 @@ main(int argc, char *argv[])
 			break;
 		case 'G':
 			grouplist = optarg;
+
+			/*
+			 * XXX Why not allow us to drop all of our supplementary
+			 * groups?
+			 */
 			if (*grouplist == '\0')
 				usage();
 			break;
@@ -139,23 +146,25 @@ main(int argc, char *argv[])
 	if (group != NULL)
 		gid = resolve_group(group);
 
-	ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1;
-	if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL)
-		err(1, "malloc");
-	/* Populate the egid slot in our groups to avoid accidents. */
-	if (gid == 0)
-		gidlist[0] = getegid();
-	else
-		gidlist[0] = gid;
-	for (gids = 1;
-	    (p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) {
-		if (*p == '\0')
-			continue;
-
-		gidlist[gids++] = resolve_group(p);
+	if (grouplist != NULL) {
+		ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1;
+		if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL)
+			err(1, "malloc");
+		/* Populate the egid slot in our groups to avoid accidents. */
+		if (gid == 0)
+			gidlist[0] = getegid();
+		else
+			gidlist[0] = gid;
+		for (gids = 1; (p = strsep(&grouplist, ",")) != NULL &&
+		    gids < ngroups_max; ) {
+			if (*p == '\0')
+				continue;
+
+			gidlist[gids++] = resolve_group(p);
+		}
+		if (p != NULL && gids == ngroups_max)
+			errx(1, "too many supplementary groups provided");
 	}
-	if (p != NULL && gids == ngroups_max)
-		errx(1, "too many supplementary groups provided");
 
 	if (user != NULL)
 		uid = resolve_user(user);
@@ -175,7 +184,7 @@ main(int argc, char *argv[])
 		err(1, "%s", argv[0]);
 	}
 
-	if (gids && setgroups(gids, gidlist) == -1)
+	if (gidlist != NULL && setgroups(gids, gidlist) == -1)
 		err(1, "setgroups");
 	if (group && setgid(gid) == -1)
 		err(1, "setgid");