kgess-mirror/freebsd-src
1
0
Fork 0
mirror of https://git.freebsd.org/src.git synced 2026-01-12 06:54:03 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

setcred(2): Fix a panic on too many groups from latest commit

Browse source 
kern_setcred_copyin_supp_groups() is documented to always set
'sc_supp_groups', but did not do it if there are more supplementary
groups than 'ngroups_max'.  Also, that case was omitted from the herald
comment.  Add it there, also including it as a case where
'sc_supp_groups_nb' is reset to 0 as a security measure.

Initially, kern_setcred_copyin_supp_groups() had the usual property that
nothing had to be freed on it returning an error, but was then converted
to relying on the caller to free() even on error, and this part was
missed during the conversion.  The benefits of this unusual convention
are that we can zero or NULLify groups-related attributes in advance,
preventing inadvertent use of stale data (defensive security measure),
and we can avoid some small code duplication (no need to have two same
calls to free()).  This makes sense as kern_setcred_copyin_supp_groups()
is meant to be a private sub-routine of user_setcred() only.  While
here, rename kern_setcred_copyin_supp_groups() =>
user_setcred_copyin_supp_groups().

Reported by:    pho
Fixes:          4cd93df95e ("setcred(): Remove an optimization for when cr_groups[0] was the egid")
Sponsored by:   The FreeBSD Foundation
This commit is contained in:
Olivier Certner 2025-11-27 10:04:50 +01:00
parent 06a4a092e6
commit 000d5b52c1
No known key found for this signature in database
GPG key ID: 8CA13040971E2627
1 changed files with 22 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
sys/kern/kern_prot.c
View file

@ -529,44 +529,54 @@ gidp_cmp(const void *p1, const void *p2)
* 'smallgroups' must be an (uninitialized) array of length CRED_SMALLGROUPS_NB.
* Always sets 'sc_supp_groups', either to a valid kernel-space groups array
* (which may or may not be 'smallgroups'), or NULL if SETCREDF_SUPP_GROUPS was
* not specified, or a buffer containing garbage on copyin() failure. In the
* last two cases, 'sc_supp_groups_nb' is additionally set to 0 as a security
* measure. 'sc_supp_groups' must be freed (M_TEMP) if not equal to
* 'smallgroups' even on failure.
* not specified or there are too many groups, or a buffer containing garbage on
* copyin() failure. In the last two cases, 'sc_supp_groups_nb' is additionally
* set to 0 as a security measure. 'sc_supp_groups' must be freed (M_TEMP) if
* not equal to 'smallgroups' even on failure.
*/
static int
kern_setcred_copyin_supp_groups(struct setcred *const wcred,
user_setcred_copyin_supp_groups(struct setcred *const wcred,
const u_int flags, gid_t *const smallgroups)
{
gid_t *groups;
int error;
if ((flags & SETCREDF_SUPP_GROUPS) == 0) {
wcred->sc_supp_groups_nb = 0;
wcred->sc_supp_groups = NULL;
return (0);
error = 0;
goto reset_groups_exit;
}
/*
* Check the number of groups' limit right now in order to limit the
* amount of bytes to copy.
*/
if (wcred->sc_supp_groups_nb > ngroups_max)
return (EINVAL);
if (wcred->sc_supp_groups_nb > ngroups_max) {
error = EINVAL;
goto reset_groups_exit;
}
groups = wcred->sc_supp_groups_nb <= CRED_SMALLGROUPS_NB ?
smallgroups : malloc(wcred->sc_supp_groups_nb * sizeof(gid_t),
M_TEMP, M_WAITOK);
error = copyin(wcred->sc_supp_groups, groups,
wcred->sc_supp_groups_nb * sizeof(gid_t));
wcred->sc_supp_groups = groups;
if (error != 0) {
wcred->sc_supp_groups_nb = 0;
/*
* 'sc_supp_groups' must be freed by caller if not
* 'smallgroups'.
*/
return (error);
}
return (0);
reset_groups_exit:
wcred->sc_supp_groups_nb = 0;
wcred->sc_supp_groups = NULL;
return (error);
}
int
@ -601,7 +611,7 @@ user_setcred(struct thread *td, const u_int flags, struct setcred *const wcred)
* alternative for 32-bit compatibility as 'gid_t' has the same size
* everywhere.
*/
error = kern_setcred_copyin_supp_groups(wcred, flags, smallgroups);
error = user_setcred_copyin_supp_groups(wcred, flags, smallgroups);
if (error != 0)
goto free_groups;