git: 580904d995d5 - main - cred: 'kern.ngroups' tunable: Limit it to avoid internal overflows

The branch main has been updated by olce:

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

commit 580904d995d53ccd2492140a37107442d8b36dc0
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-10-01 17:00:43 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-11-02 20:37:41 +0000

    cred: 'kern.ngroups' tunable: Limit it to avoid internal overflows
    
    As the comment introduced with the tunable said (but the code didn't
    do), make sure that 'ngroups_max' can't be INT_MAX, as this would cause
    overflow in the usual 'ngroups_max + 1' computations (as we store the
    effective GID and supplementary groups' IDs in the same array, and
    'ngroups_max' only applies to supplementary groups).
    
    Further, we limit the maximum number of groups somewhat arbitrarily to
    ~17M so as to avoid overflow when computing the size in bytes of the
    groups set's backing array and to avoid obvious configuration errors.
    We really don't think that more than ~17M groups will ever be needed (if
    I'm proven wrong one day, please drop me a note about your use case).
    
    While here, document more precisely why NGROUPS_MAX needs to be the
    minimum value for 'ngroups_max'.
    
    Reviewed by:    mhorne (older version)
    Approved by:    markj (mentor)
    MFC after:      3 days
    Differential Revision:  https://reviews.freebsd.org/D46913
---
 sys/kern/subr_param.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/sys/kern/subr_param.c b/sys/kern/subr_param.c
index 7d58208a461f..19169ba63061 100644
--- a/sys/kern/subr_param.c
+++ b/sys/kern/subr_param.c
@@ -228,14 +228,32 @@ init_param1(void)
 	TUNABLE_ULONG_FETCH("kern.sgrowsiz", &sgrowsiz);
 
 	/*
-	 * Let the administrator set {NGROUPS_MAX}, but disallow values
-	 * less than NGROUPS_MAX which would violate POSIX.1-2008 or
-	 * greater than INT_MAX-1 which would result in overflow.
+	 * Let the administrator set {NGROUPS_MAX}.
+	 *
+	 * Values less than NGROUPS_MAX would violate POSIX/SuS (see the
+	 * specification for <limits.h>, paragraph "Runtime Increasable
+	 * Values").
+	 *
+	 * On the other hand, INT_MAX would result in an overflow for the common
+	 * 'ngroups_max + 1' computation (to obtain the size of the internal
+	 * groups array, its first element being reserved for the effective
+	 * GID).  Also, the number of allocated bytes for the group array must
+	 * not overflow on 32-bit machines.  For all these reasons, we limit the
+	 * number of supplementary groups to some very high number that we
+	 * expect will never be reached in all practical uses and ensures we
+	 * avoid the problems just exposed, even if 'gid_t' was to be enlarged
+	 * by a magnitude.
 	 */
 	ngroups_max = NGROUPS_MAX;
 	TUNABLE_INT_FETCH("kern.ngroups", &ngroups_max);
 	if (ngroups_max < NGROUPS_MAX)
 		ngroups_max = NGROUPS_MAX;
+	else {
+		const int ngroups_max_max = (1 << 24) - 1;
+
+		if (ngroups_max > ngroups_max_max)
+			ngroups_max = ngroups_max_max;
+	}
 
 	/*
 	 * Only allow to lower the maximal pid.