Page Menu
Home
FreeBSD
Search
Configure Global Search
Log In
Files
F168413444
D51648.id159740.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Flag For Later
Award Token
Size
11 KB
Referenced Files
None
Subscribers
None
D51648.id159740.diff
View Options
diff --git a/lib/libsys/getgroups.2 b/lib/libsys/getgroups.2
--- a/lib/libsys/getgroups.2
+++ b/lib/libsys/getgroups.2
@@ -25,7 +25,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd January 21, 2011
+.Dd August 1, 2025
.Dt GETGROUPS 2
.Os
.Sh NAME
@@ -41,8 +41,8 @@
The
.Fn getgroups
system call
-gets the current group access list of the user process
-and stores it in the array
+gets the current supplementary groups of the user process and stores it in the
+array
.Fa gidset .
The
.Fa gidsetlen
@@ -54,7 +54,7 @@
system call
returns the actual number of groups returned in
.Fa gidset .
-At least one and as many as {NGROUPS_MAX}+1 values may be returned.
+As many as {NGROUPS_MAX} values may be returned.
If
.Fa gidsetlen
is zero,
@@ -102,3 +102,10 @@
.Fn getgroups
system call appeared in
.Bx 4.2 .
+.Pp
+Before
+.Fx 15.0 ,
+the
+.Fn getgroups
+system call always returned the effective group ID for the process as the first
+element of the array, before the supplementary groups.
diff --git a/lib/libsys/setgroups.2 b/lib/libsys/setgroups.2
--- a/lib/libsys/setgroups.2
+++ b/lib/libsys/setgroups.2
@@ -25,7 +25,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd January 19, 2018
+.Dd August 1, 2025
.Dt SETGROUPS 2
.Os
.Sh NAME
@@ -42,7 +42,7 @@
The
.Fn setgroups
system call
-sets the group access list of the current user process
+sets the supplementary group list of the current user process
according to the array
.Fa gidset .
The
@@ -50,26 +50,12 @@
argument
indicates the number of entries in the array and must be no
more than
-.Dv {NGROUPS_MAX}+1 .
-.Pp
-Only the super-user may set a new group list.
+.Dv {NGROUPS_MAX} .
+The
+.Fa ngroups
+argument may be set to 0 to clear the supplementary group list.
.Pp
-The first entry of the group array
-.Pq Va gidset[0]
-is used as the effective group-ID for the process.
-This entry is over-written when a setgid program is run.
-To avoid losing access to the privileges of the
-.Va gidset[0]
-entry, it should be duplicated later in the group array.
-By convention,
-this happens because the group value indicated
-in the password file also appears in
-.Pa /etc/group .
-The group value in the password file is placed in
-.Va gidset[0]
-and that value then gets added a second time when the
-.Pa /etc/group
-file is scanned to create the group set.
+Only the super-user may set a new supplementary group list.
.Sh RETURN VALUES
.Rv -std setgroups
.Sh ERRORS
@@ -99,3 +85,11 @@
.Fn setgroups
system call appeared in
.Bx 4.2 .
+.Pp
+Before
+.Fx 15.0 ,
+the
+.Fn setgroups
+system call would set the effective group ID for the process to the first
+element of
+.Fa gidset .
diff --git a/sbin/hastd/subr.c b/sbin/hastd/subr.c
--- a/sbin/hastd/subr.c
+++ b/sbin/hastd/subr.c
@@ -207,10 +207,8 @@
}
}
PJDLOG_VERIFY(chdir("/") == 0);
- gidset[0] = pw->pw_gid;
- if (setgroups(1, gidset) == -1) {
- pjdlog_errno(LOG_ERR, "Unable to set groups to gid %u",
- (unsigned int)pw->pw_gid);
+ if (setgroups(0, NULL) == -1) {
+ pjdlog_errno(LOG_ERR, "Unable to drop supplementary groups");
return (-1);
}
if (setgid(pw->pw_gid) == -1) {
@@ -287,8 +285,7 @@
PJDLOG_VERIFY(egid == pw->pw_gid);
PJDLOG_VERIFY(sgid == pw->pw_gid);
PJDLOG_VERIFY(getgroups(0, NULL) == 1);
- PJDLOG_VERIFY(getgroups(1, gidset) == 1);
- PJDLOG_VERIFY(gidset[0] == pw->pw_gid);
+ PJDLOG_VERIFY(getgroups(1, gidset) == 0);
pjdlog_debug(1,
"Privileges successfully dropped using %s%s+setgid+setuid.",
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -320,18 +320,20 @@
sys_getgroups(struct thread *td, struct getgroups_args *uap)
{
struct ucred *cred;
- gid_t *ugidset;
int ngrp, error;
+#ifdef COMPAT_FREEBSD14
+ int osrel = td->td_proc->p_osrel;
+ bool getgroups_compat;
+#endif
cred = td->td_ucred;
- /*
- * cr_gid has been moved out of cr_groups, but we'll continue exporting
- * the egid as groups[0] for the time being until we audit userland for
- * any surprises.
- */
- ngrp = cred->cr_ngroups + 1;
-
+ ngrp = cred->cr_ngroups;
+#ifdef COMPAT_FREEBSD14
+ getgroups_compat = osrel < P_OSREL_SETGROUPS_EGID;
+ if (getgroups_compat)
+ ngrp++;
+#endif
if (uap->gidsetsize == 0) {
error = 0;
goto out;
@@ -339,14 +341,15 @@
if (uap->gidsetsize < ngrp)
return (EINVAL);
- ugidset = uap->gidset;
- error = copyout(&cred->cr_gid, ugidset, sizeof(*ugidset));
- if (error != 0)
- goto out;
-
- if (ngrp > 1)
- error = copyout(cred->cr_groups, ugidset + 1,
- (ngrp - 1) * sizeof(*ugidset));
+#ifdef COMPAT_FREEBSD14
+ if (getgroups_compat) {
+ error = copyout(&cred->cr_gid, uap->gidset, sizeof(gid_t));
+ if (error != 0)
+ error = copyout(cred->cr_groups, uap->gidset + 1,
+ (ngrp - 1) * sizeof(gid_t));
+ } else
+#endif
+ error = copyout(cred->cr_groups, uap->gidset, ngrp * sizeof(gid_t));
out:
td->td_retval[0] = ngrp;
return (error);
@@ -1198,7 +1201,18 @@
{
gid_t smallgroups[CRED_SMALLGROUPS_NB];
gid_t *groups;
- int gidsetsize, error;
+ int gidsetsize, error, maxsize;
+#ifdef COMPAT_FREEBSD14
+ int osrel = td->td_proc->p_osrel;
+ bool setgroups_compat;
+#endif
+
+ maxsize = ngroups_max;
+#ifdef COMPAT_FREEBSD14
+ setgroups_compat = osrel < P_OSREL_SETGROUPS_EGID;
+ if (setgroups_compat)
+ maxsize++; /* One more to account for the egid */
+#endif
/*
* Sanity check size now to avoid passing too big a value to copyin(),
@@ -1210,8 +1224,7 @@
* setgroups() differ.
*/
gidsetsize = uap->gidsetsize;
- /* XXXKE Limit to ngroups_max when we change the userland interface. */
- if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
+ if (gidsetsize > maxsize || gidsetsize < 0)
return (EINVAL);
if (gidsetsize > CRED_SMALLGROUPS_NB)
@@ -1220,8 +1233,19 @@
groups = smallgroups;
error = copyin(uap->gidset, groups, gidsetsize * sizeof(gid_t));
- if (error == 0)
+ if (error == 0) {
+#ifdef COMPAT_FREEBSD14
+ if (gidsetsize > 0 && setgroups_compat) {
+ gidsetsize--;
+ error = kern_setgroups(td, &gidsetsize, groups + 1);
+ } else
+#endif
error = kern_setgroups(td, &gidsetsize, groups);
+#ifdef COMPAT_FREEBSD14
+ if (uap->gidsetsize > 0 && error == 0 && setgroups_compat)
+ td->td_proc->p_ucred->cr_gid = groups[0];
+#endif
+ }
if (groups != smallgroups)
free(groups, M_TEMP);
@@ -1238,35 +1262,17 @@
struct proc *p = td->td_proc;
struct ucred *newcred, *oldcred;
int ngrp, error;
- gid_t egid;
ngrp = *ngrpp;
/* Sanity check size. */
- /* XXXKE Limit to ngroups_max when we change the userland interface. */
- if (ngrp < 0 || ngrp > ngroups_max + 1)
+ if (ngrp < 0 || ngrp > ngroups_max)
return (EINVAL);
AUDIT_ARG_GROUPSET(groups, ngrp);
- /*
- * setgroups(0, NULL) is a legitimate way of clearing the groups vector
- * on non-BSD systems (which generally do not have the egid in the
- * groups[0]). We risk security holes when running non-BSD software if
- * we do not do the same. So we allow and treat 0 for 'ngrp' specially
- * below (twice).
- */
- if (ngrp != 0) {
- /*
- * To maintain userland compat for now, we use the first group
- * as our egid and we'll use the rest as our supplemental
- * groups.
- */
- egid = groups[0];
- ngrp--;
- groups++;
- groups_normalize(&ngrp, groups);
- *ngrpp = ngrp;
- }
+ groups_normalize(&ngrp, groups);
+ *ngrpp = ngrp;
+
newcred = crget();
crextend(newcred, ngrp);
PROC_LOCK(p);
@@ -1289,15 +1295,7 @@
if (error)
goto fail;
- /*
- * If some groups were passed, the first one is currently the desired
- * egid. This code is to be removed (along with some commented block
- * above) when setgroups() is changed to take only supplementary groups.
- */
- if (ngrp != 0)
- newcred->cr_gid = egid;
crsetgroups_internal(newcred, ngrp, groups);
-
setsugid(p);
proc_set_cred(p, newcred);
PROC_UNLOCK(p);
diff --git a/sys/sys/param.h b/sys/sys/param.h
--- a/sys/sys/param.h
+++ b/sys/sys/param.h
@@ -109,6 +109,7 @@
#define P_OSREL_ARM64_SPSR 1400084
#define P_OSREL_TLSBASE 1500044
#define P_OSREL_EXTERRCTL 1500045
+#define P_OSREL_SETGROUPS_EGID 1500056
#define P_OSREL_MAJOR(x) ((x) / 100000)
#endif
diff --git a/usr.bin/newgrp/newgrp.c b/usr.bin/newgrp/newgrp.c
--- a/usr.bin/newgrp/newgrp.c
+++ b/usr.bin/newgrp/newgrp.c
@@ -186,7 +186,7 @@
}
}
- ngrps_max = sysconf(_SC_NGROUPS_MAX) + 1;
+ ngrps_max = sysconf(_SC_NGROUPS_MAX);
if ((grps = malloc(sizeof(gid_t) * ngrps_max)) == NULL)
err(1, "malloc");
if ((ngrps = getgroups(ngrps_max, (gid_t *)grps)) < 0) {
@@ -194,7 +194,12 @@
goto end;
}
- /* Remove requested gid from supp. list if it exists. */
+ /*
+ * Remove requested gid from supp. list if it exists and doesn't match
+ * our prior egid -- this exception is to avoid providing the user a
+ * means to get rid of a group that could be used for, e.g., negative
+ * permissions.
+ */
if (grp->gr_gid != egid && inarray(grp->gr_gid, grps, ngrps)) {
for (i = 0; i < ngrps; i++)
if (grps[i] == grp->gr_gid)
@@ -217,10 +222,9 @@
goto end;
}
PRIV_END;
- grps[0] = grp->gr_gid;
/* Add old effective gid to supp. list if it does not exist. */
- if (egid != grp->gr_gid && !inarray(egid, grps, ngrps)) {
+ if (!inarray(egid, grps, ngrps)) {
if (ngrps == ngrps_max)
warnx("too many groups");
else {
diff --git a/usr.bin/quota/quota.c b/usr.bin/quota/quota.c
--- a/usr.bin/quota/quota.c
+++ b/usr.bin/quota/quota.c
@@ -100,8 +100,7 @@
int
main(int argc, char *argv[])
{
- int ngroups;
- gid_t mygid, gidset[NGROUPS];
+ int ngroups;
int i, ch, gflag = 0, uflag = 0, errflag = 0;
while ((ch = getopt(argc, argv, "f:ghlrquv")) != -1) {
@@ -142,11 +141,15 @@
if (uflag)
errflag += showuid(getuid());
if (gflag) {
+ gid_t mygid, myegid, gidset[NGROUPS_MAX];
+
mygid = getgid();
- ngroups = getgroups(NGROUPS, gidset);
+ errflag += showgid(mygid);
+ myegid = getegid();
+ errflag += showgid(myegid);
+ ngroups = getgroups(NGROUPS_MAX, gidset);
if (ngroups < 0)
err(1, "getgroups");
- errflag += showgid(mygid);
for (i = 0; i < ngroups; i++)
if (gidset[i] != mygid)
errflag += showgid(gidset[i]);
diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c
--- a/usr.sbin/chroot/chroot.c
+++ b/usr.sbin/chroot/chroot.c
@@ -139,15 +139,10 @@
if (group != NULL)
gid = resolve_group(group);
- ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1;
+ ngroups_max = sysconf(_SC_NGROUPS_MAX);
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;
+ for (gids = 0;
(p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) {
if (*p == '\0')
continue;
diff --git a/usr.sbin/lpr/lpc/lpc.c b/usr.sbin/lpr/lpc/lpc.c
--- a/usr.sbin/lpr/lpc/lpc.c
+++ b/usr.sbin/lpr/lpc/lpc.c
@@ -358,6 +358,8 @@
err(1, "getgroups");
}
gid = gptr->gr_gid;
+ if (gid == getegid())
+ return(1);
for (i = 0; i < ngroups; i++)
if (gid == groups[i])
return(1);
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Aug 29, 3:50 AM (17 h, 58 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
37495120
Default Alt Text
D51648.id159740.diff (11 KB)
Attached To
Mode
D51648: kern: fix setgroups(2) and getgroups(2) to match other platforms
Attached
Detach File
Event Timeline
Log In to Comment