The branch main has been updated by kevans:
URL:
https://cgit.FreeBSD.org/src/commit/?id=babab49eee9472f628d774996de13d13d296c8c0
commit babab49eee9472f628d774996de13d13d296c8c0
Author: Kyle Evans <kev...@freebsd.org>
AuthorDate: 2025-08-12 12:14:38 +0000
Commit: Kyle Evans <kev...@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");
