git: 5d10a05c6dfa - stable/14 - cred: kern_setgroups(): Internally use int as number of groups' type

The branch stable/14 has been updated by olce:

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

commit 5d10a05c6dfacd30a2691e92eb68cfabb6e94104
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-10-01 16:46:46 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-11-15 10:47:41 +0000

    cred: kern_setgroups(): Internally use int as number of groups' type
    
    sys_setgroups() (and sys_getgroups()) was changed in commit "kern: fail
    getgroup and setgroup with negative int" (4bc2174a1b48) to take the
    number of groups as an 'int' (for sys_getgroups(), POSIX mandates this
    change; for sys_setgroups(), which it does not standardize, it's
    arguably for consistency).
    
    All our internal APIs related to groups on 'struct ucred', as well as
    related members on the latter, treat that number as an 'int' as well
    (and not a 'u_int').
    
    Consequently, to avoid surprises, change kern_setgroups() to behave the
    same, and fix audit_arg_groupset() accordingly.  With that change,
    everything is handled with signed integers internally.
    
    Update sanity checks accordingly.
    
    Reviewed by:    mhorne
    Approved by:    markj (mentor)
    MFC after:      3 days
    Differential Revision:  https://reviews.freebsd.org/D46912
    
    (cherry picked from commit abd39811cd7e4bb928da503f4a5c79364ac8d0f5)
    
    Approved by:    markj (mentor)
---
 sys/kern/kern_prot.c           | 16 ++++++++++++++--
 sys/security/audit/audit.h     |  2 +-
 sys/security/audit/audit_arg.c |  8 ++++----
 sys/sys/syscallsubr.h          |  2 +-
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index dc2ab35dc5df..3052dc44d828 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -815,6 +815,15 @@ sys_setgroups(struct thread *td, struct setgroups_args *uap)
 	gid_t *groups;
 	int gidsetsize, error;
 
+	/*
+	 * Sanity check size now to avoid passing too big a value to copyin(),
+	 * even if kern_setgroups() will do it again.
+	 *
+	 * Ideally, the 'gidsetsize' argument should have been a 'u_int' (and it
+	 * was, in this implementation, for a long time), but POSIX standardized
+	 * getgroups() to take an 'int' and it would be quite entrapping to have
+	 * setgroups() differ.
+	 */
 	gidsetsize = uap->gidsetsize;
 	if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
 		return (EINVAL);
@@ -843,13 +852,16 @@ gidp_cmp(const void *p1, const void *p2)
 }
 
 int
-kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups)
+kern_setgroups(struct thread *td, int ngrp, gid_t *groups)
 {
 	struct proc *p = td->td_proc;
 	struct ucred *newcred, *oldcred;
 	int error;
 
-	MPASS(ngrp <= ngroups_max + 1);
+	/* Sanity check size. */
+	if (ngrp < 0 || ngrp > ngroups_max + 1)
+		return (EINVAL);
+
 	AUDIT_ARG_GROUPSET(groups, ngrp);
 	newcred = crget();
 	crextend(newcred, ngrp);
diff --git a/sys/security/audit/audit.h b/sys/security/audit/audit.h
index e7a9c83afbb3..b87dd52e0773 100644
--- a/sys/security/audit/audit.h
+++ b/sys/security/audit/audit.h
@@ -98,7 +98,7 @@ void	 audit_arg_rgid(gid_t rgid);
 void	 audit_arg_ruid(uid_t ruid);
 void	 audit_arg_sgid(gid_t sgid);
 void	 audit_arg_suid(uid_t suid);
-void	 audit_arg_groupset(gid_t *gidset, u_int gidset_size);
+void	 audit_arg_groupset(gid_t *gidset, int gidset_size);
 void	 audit_arg_login(char *login);
 void	 audit_arg_ctlname(int *name, int namelen);
 void	 audit_arg_mask(int mask);
diff --git a/sys/security/audit/audit_arg.c b/sys/security/audit/audit_arg.c
index 7b7fe34ccda9..db621db1574f 100644
--- a/sys/security/audit/audit_arg.c
+++ b/sys/security/audit/audit_arg.c
@@ -264,13 +264,13 @@ audit_arg_suid(uid_t suid)
 }
 
 void
-audit_arg_groupset(gid_t *gidset, u_int gidset_size)
+audit_arg_groupset(gid_t *gidset, int gidset_size)
 {
-	u_int i;
+	int i;
 	struct kaudit_record *ar;
 
-	KASSERT(gidset_size <= ngroups_max + 1,
-	    ("audit_arg_groupset: gidset_size > (kern.ngroups + 1)"));
+	KASSERT(gidset_size >= 0 && gidset_size <= ngroups_max + 1,
+	    ("audit_arg_groupset: gidset_size < 0 or > (kern.ngroups + 1)"));
 
 	ar = currecord();
 	if (ar == NULL)
diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h
index 87e29f5ca5fc..1b34a3db8223 100644
--- a/sys/sys/syscallsubr.h
+++ b/sys/sys/syscallsubr.h
@@ -312,7 +312,7 @@ int	kern_select(struct thread *td, int nd, fd_set *fd_in, fd_set *fd_ou,
 	    fd_set *fd_ex, struct timeval *tvp, int abi_nfdbits);
 int	kern_sendit(struct thread *td, int s, struct msghdr *mp, int flags,
 	    struct mbuf *control, enum uio_seg segflg);
-int	kern_setgroups(struct thread *td, u_int ngrp, gid_t *groups);
+int	kern_setgroups(struct thread *td, int ngrp, gid_t *groups);
 int	kern_setitimer(struct thread *, u_int, struct itimerval *,
 	    struct itimerval *);
 int	kern_setpriority(struct thread *td, int which, int who, int prio);