git: e6fbef451dd4 - main - cred: Fix a race in the FreeBSD-14-compatible setgroups(2)

The branch main has been updated by olce:

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

commit e6fbef451dd45159071a1b234cd13c66fbb654fa
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2026-09-25 17:27:25 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2026-09-30 09:05:02 +0000

    cred: Fix a race in the FreeBSD-14-compatible setgroups(2)
    
    The freebsd14_setgroups() function would try to modify the effective GID
    on the current process' credentials without holding the process lock,
    allowing races with other threads concurrently modifying the process
    credentials.  In the worst case, freebsd14_setgroups() could be
    manipulating a 'struct ucred' already freed by another thread (in the
    very small window after reading 'p_ucred' without lock but before
    modifying the effective GID).  Concurrent uses of freebsd14_setgroups()
    or setcred() could also lead to non-atomic credentials modifications.
    
    Fix this by making kern_setgroups() take a new boolean indicating
    whether the passed array includes the effective GID in its first slot.
    When this boolean is true, it internally keeps the effective GID in
    a separate variable, pretends that the groups[] array that was passed
    actually starts at 'groups + 1', do the usual steps to set the
    supplementary groups and new extra ones to set the effective GID along,
    without releasing the process lock in between.
    
    Reported by:    markj
    Reviewed by:    markj
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D60028
---
 sys/kern/kern_prot.c  | 58 ++++++++++++++++++++++++++++++++++++++-------------
 sys/sys/syscallsubr.h |  3 ++-
 2 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 5370028f4490..bfb95508b64e 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1219,9 +1219,7 @@ freebsd14_setgroups(struct thread *td, struct freebsd14_setgroups_args *uap)
 
 	/*
 	 * Before FreeBSD 15.0, we allow one more group to be supplied to
-	 * account for the egid appearing before the supplementary groups.  This
-	 * may technically allow one more supplementary group for systems that
-	 * did use the default NGROUPS_MAX if we round it back up to 1024.
+	 * account for the egid appearing before the supplementary groups.
 	 */
 	gidsetsize = uap->gidsetsize;
 	if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
@@ -1234,11 +1232,9 @@ freebsd14_setgroups(struct thread *td, struct freebsd14_setgroups_args *uap)
 
 	error = copyin(uap->gidset, groups, gidsetsize * sizeof(gid_t));
 	if (error == 0) {
-		int ngroups = gidsetsize > 0 ? gidsetsize - 1 /* egid */ : 0;
+		int ngroups = gidsetsize;
 
-		error = kern_setgroups(td, &ngroups, groups + 1);
-		if (error == 0 && gidsetsize > 0)
-			td->td_proc->p_ucred->cr_gid = groups[0];
+		error = kern_setgroups(td, &ngroups, groups, true);
 	}
 
 	if (groups != smallgroups)
@@ -1281,7 +1277,7 @@ sys_setgroups(struct thread *td, struct setgroups_args *uap)
 
 	error = copyin(uap->gidset, groups, gidsetsize * sizeof(gid_t));
 	if (error == 0)
-		error = kern_setgroups(td, &gidsetsize, groups);
+		error = kern_setgroups(td, &gidsetsize, groups, false);
 
 	if (groups != smallgroups)
 		free(groups, M_TEMP);
@@ -1289,25 +1285,43 @@ sys_setgroups(struct thread *td, struct setgroups_args *uap)
 }
 
 /*
- * CAUTION: This function normalizes 'groups', possibly also changing the value
- * of '*ngrpp' as a consequence.
+ * 'includes_egid' indicates that the first element of groups[] (if any) is the
+ * desired effective GID and that only the other elements will be used to set
+ * the supplementary groups.  If true, and groups[] is empty, the effective GID
+ * is left unchanged and all supplementary groups deleted (see setgroups(2)).
+ *
+ * CAUTION: This function normalizes 'groups' (only the supplementary groups on
+ * 'includes_egid') and may need to update the value of '*ngrpp' as
+ * a consequence.
  */
 int
-kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups)
+kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups, bool includes_egid)
 {
 	struct proc *p = td->td_proc;
 	struct ucred *newcred, *oldcred;
+	gid_t egid;
 	int ngrp, error;
 
 	ngrp = *ngrpp;
 	/* Sanity check size. */
-	if (ngrp < 0 || ngrp > ngroups_max)
+	if (ngrp < 0 || ngrp > (includes_egid ? ngroups_max + 1 : ngroups_max))
 		return (EINVAL);
 
+	if (includes_egid) {
+		if (ngrp > 0) {
+			egid = groups[0];
+			groups++;
+			ngrp--;
+		} else
+			includes_egid = false;
+	}
+
 	AUDIT_ARG_GROUPSET(groups, ngrp);
+	if (includes_egid)
+		AUDIT_ARG_EGID(egid);
 
 	groups_normalize(&ngrp, groups);
-	*ngrpp = ngrp;
+	*ngrpp = includes_egid ? ngrp + 1 : ngrp;
 
 	newcred = crget();
 	crextend(newcred, ngrp);
@@ -1324,15 +1338,29 @@ kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups)
 	 */
 	error = mac_cred_check_setgroups(oldcred, ngrp,
 	    ngrp == 0 ? NULL : groups);
-	if (error)
+	if (error != 0)
 		goto fail;
+
+	if (includes_egid) {
+		error = mac_cred_check_setegid(oldcred, egid);
+		if (error != 0)
+			goto fail;
+	}
 #endif
 
 	error = priv_check_cred(oldcred, PRIV_CRED_SETGROUPS);
-	if (error)
+	if (error != 0)
 		goto fail;
 
+	if (includes_egid) {
+		error = priv_check_cred(oldcred, PRIV_CRED_SETEGID);
+		if (error != 0)
+			goto fail;
+	}
+
 	crsetgroups_internal(newcred, ngrp, groups);
+	if (includes_egid)
+		change_egid(newcred, egid);
 	setsugid(p);
 	proc_set_cred(p, newcred);
 	PROC_UNLOCK(p);
diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h
index d767ba29cdf6..d55426cf7d98 100644
--- a/sys/sys/syscallsubr.h
+++ b/sys/sys/syscallsubr.h
@@ -365,7 +365,8 @@ int	kern_sendit(struct thread *td, int s, struct msghdr *mp, int flags,
 	    struct mbuf *control, enum uio_seg segflg);
 int	kern_setcred(struct thread *const td, const u_int flags,
 	    struct setcred *const wcred);
-int	kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups);
+int	kern_setgroups(struct thread *td, int *ngrpp, gid_t *groups,
+	    bool includes_egid);
 int	kern_setitimer(struct thread *, u_int, struct itimerval *,
 	    struct itimerval *);
 int	kern_setpriority(struct thread *td, int which, int who, int prio);