Re: git: 8878569103a3 - stable/15 - initgroups(3): Add a pre-FreeBSD-15-compatible version
Date: Tue, 23 Sep 2025 15:24:18 UTC
On 9/23/25 07:03, Olivier Certner wrote:
> The branch stable/15 has been updated by olce:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=8878569103a382aa2c7142c203285a50484fe298
>
> commit 8878569103a382aa2c7142c203285a50484fe298
> Author: Olivier Certner <olce@FreeBSD.org>
> AuthorDate: 2025-08-29 14:19:33 +0000
> Commit: Olivier Certner <olce@FreeBSD.org>
> CommitDate: 2025-09-23 12:02:46 +0000
>
> initgroups(3): Add a pre-FreeBSD-15-compatible version
>
> After commit 9da2fe96ff2e ("kern: fix setgroups(2) and getgroups(2) to
> match other platforms"), initgroups() does not set the effective GID
> anymore and uses all passed groups as the supplementary group list.
> This effectively breaks backwards compatibility with programs/libraries
> compiled on a FreeBSD 14 or earlier system.
>
> Restore compatibility by creating a new version of the 'initgroups'
> symbol that designates the current implementation and providing
> a pre-FreeBSD-15-compatible version under the symbol's previously
> exported version. The new version calls the new setgroups(2) system
> call, while the compatible one calls the original one (called
> freebsd14_setgroups()).
>
> Update the manual page with some history and comparison with other
> current open-source systems. Add a "SECURITY CONSIDERATIONS" section
> highlighting some security properties of this approach and the reasons
> we adopt it. While here, revamp the manual page, in particular to use
> the exact POSIX terminology where possible.
>
> Note for MFC to stable/14: Only the manual page update is to be MFCed,
> and the text changed to reflect the old behavior and inform readers of
> the new upcoming behavior in 15.
>
> Reviewed by: kib
> Fixes: 9da2fe96ff2e ("kern: fix setgroups(2) and getgroups(2) to match other platforms")
> MFC after: 5 days
> Sponsored by: The FreeBSD Foundation
> Differential Revision: https://reviews.freebsd.org/D52282
>
> (cherry picked from commit 9dc1ac8691966480ff8bd9c37dd405b981b41dd5)
This one should not have been MFC'd without the follow-up that fixes initgroups@FBSD_1.0,
please back it out or MFC the fix ASAP- it doesn't make any sense to leave stable/15 broken
like this when we knew there was a problem.
Thanks,
Kyle Evans
> ---
> lib/libc/gen/Symbol.map | 2 +-
> lib/libc/gen/gen-compat.h | 2 +
> lib/libc/gen/initgroups.3 | 101 +++++++++++++++++++++++++++++++++++++++-------
> lib/libc/gen/initgroups.c | 44 +++++++++++++++-----
> 4 files changed, 124 insertions(+), 25 deletions(-)
>
> diff --git a/lib/libc/gen/Symbol.map b/lib/libc/gen/Symbol.map
> index 26f638568efc..494b65bc5cc1 100644
> --- a/lib/libc/gen/Symbol.map
> +++ b/lib/libc/gen/Symbol.map
> @@ -193,7 +193,6 @@ FBSD_1.0 {
> __isinff;
> __isinfl;
> isatty;
> - initgroups;
> jrand48;
> lcong48;
> ldexp;
> @@ -462,6 +461,7 @@ FBSD_1.8 {
> fdscandir_b;
> fts_open_b;
> glob_b;
> + initgroups;
> inotify_add_watch;
> inotify_init;
> inotify_init1;
> diff --git a/lib/libc/gen/gen-compat.h b/lib/libc/gen/gen-compat.h
> index 08e80ede6b6e..dac8f54b45a2 100644
> --- a/lib/libc/gen/gen-compat.h
> +++ b/lib/libc/gen/gen-compat.h
> @@ -52,4 +52,6 @@ int freebsd11_getmntinfo(struct freebsd11_statfs **, int);
> char *freebsd11_devname(__uint32_t dev, __mode_t type);
> char *freebsd11_devname_r(__uint32_t dev, __mode_t type, char *buf, int len);
>
> +int freebsd14_setgroups(int gidsize, const __gid_t *gidset);
> +
> #endif /* _GEN_COMPAT_H_ */
> diff --git a/lib/libc/gen/initgroups.3 b/lib/libc/gen/initgroups.3
> index 03bd07494fc9..4f538fb180ec 100644
> --- a/lib/libc/gen/initgroups.3
> +++ b/lib/libc/gen/initgroups.3
> @@ -1,5 +1,13 @@
> +.\"-
> +.\" SPDX-License-Identifier: BSD-3-Clause
> +.\"
> .\" Copyright (c) 1983, 1991, 1993
> .\" The Regents of the University of California. All rights reserved.
> +.\" Copyright (c) 2025 The FreeBSD Foundation
> +.\"
> +.\" Portions of this documentation were written by Olivier Certner
> +.\" <olce@FreeBSD.org> at Kumacom SARL under sponsorship from the FreeBSD
> +.\" Foundation.
> .\"
> .\" Redistribution and use in source and binary forms, with or without
> .\" modification, are permitted provided that the following conditions
> @@ -25,12 +33,12 @@
> .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> .\" SUCH DAMAGE.
> .\"
> -.Dd October 26, 2014
> +.Dd September 17, 2025
> .Dt INITGROUPS 3
> .Os
> .Sh NAME
> .Nm initgroups
> -.Nd initialize group access list
> +.Nd initialize supplementary groups as per the group database
> .Sh LIBRARY
> .Lb libc
> .Sh SYNOPSIS
> @@ -40,19 +48,18 @@
> .Sh DESCRIPTION
> The
> .Fn initgroups
> -function
> -uses the
> -.Xr getgrouplist 3
> -function to calculate the group access list for the user
> -specified in
> +function initializes the current process' supplementary groups as prescribed by
> +its arguments and the system's group database.
> +.Pp
> +It first uses the
> +.Fn getgrouplist
> +function to compute a list of groups containing the passed
> +.Fa basegid ,
> +which typically is the user's initial numerical group ID from the password
> +database, and the supplementary groups in the group database for the user named
> .Fa name .
> -This group list is then setup for the current process using
> -.Xr setgroups 2 .
> -The
> -.Fa basegid
> -is automatically included in the groups list.
> -Typically this value is given as
> -the group number from the password file.
> +It then installs this list as the current process' supplementary groups using
> +.Fn setgroups .
> .Sh RETURN VALUES
> .Rv -std initgroups
> .Sh ERRORS
> @@ -60,7 +67,7 @@ The
> .Fn initgroups
> function may fail and set
> .Va errno
> -for any of the errors specified for the library function
> +to any of the errors specified for the library function
> .Xr setgroups 2 .
> It may also return:
> .Bl -tag -width Er
> @@ -77,3 +84,67 @@ The
> .Fn initgroups
> function appeared in
> .Bx 4.2 .
> +.Pp
> +The
> +.Fn initgroups
> +function changed semantics in
> +.Fx 15 ,
> +following that of
> +.Xr setgroups 2
> +in the same release.
> +Before that, it would also set the effective group ID to
> +.Fa basegid ,
> +and would not include the latter in the supplementary groups except before
> +.Fx 8 .
> +Its current behavior in these respects is known to be compatible with that of
> +the following systems up to the specified versions that are current at time of
> +this writing:
> +.Bl -dash -width "-" -compact
> +.It
> +Linux (up to 6.6) with the GNU libc (up to 2.42)
> +.It
> +.Nx 1.1 and greater (up to 10)
> +.It
> +.Ox (up to 7.7)
> +.It
> +Systems based on illumos (up to August 2025 sources)
> +.El
> +.Sh SECURITY CONSIDERATIONS
> +As
> +.Fa basegid
> +is typically the user's initial numerical group ID, to which the current
> +process' effective group ID is generally initialized, processes using functions
> +to change their effective group ID
> +.Pq via Xr setgid 2 or similar
> +or that are spawned from executables with the set-group-ID mode bit set will not
> +be able to relinquish the access rights deriving from being a member of
> +.Fa basegid ,
> +as these functions do not change the supplementary groups.
> +.Pp
> +This behavior is generally desirable in order to paper over the difference of
> +treatment between the effective group and supplementary ones in this situation,
> +as they are all in the end indiscriminately used in traditional UNIX
> +discretionary access checks.
> +It blends well with the practice of allocating each user its own private group,
> +as processes launched from a set-group-ID executable keep the same user and
> +consistently stay also in the same user's group.
> +Finally, it was also chosen for compatibility with other systems
> +.Po
> +see the
> +.Sx HISTORY
> +section
> +.Pc .
> +.Pp
> +This convention of including
> +.Fa basegid
> +in the supplementary groups is however only enforced by the
> +.Fn initgroups
> +function, and not by the
> +.Xr setgroups 2
> +system call, so applications expressly wanting to include in the supplementary
> +groups only those specified by the group database can themselves call
> +.Fn getgrouplist
> +and then
> +.Fn setgroups
> +on the result with the first element skipped
> +.Pq see Xr getgrouplist 3 .
> diff --git a/lib/libc/gen/initgroups.c b/lib/libc/gen/initgroups.c
> index 55f17a94fa8e..a1a7d92250e2 100644
> --- a/lib/libc/gen/initgroups.c
> +++ b/lib/libc/gen/initgroups.c
> @@ -3,6 +3,11 @@
> *
> * Copyright (c) 1983, 1993
> * The Regents of the University of California. All rights reserved.
> + * Copyright (c) 2025 The FreeBSD Foundation
> + *
> + * Portions of this software were developed by Olivier Certner
> + * <olce@FreeBSD.org> at Kumacom SARL under sponsorship from the FreeBSD
> + * Foundation.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions
> @@ -29,22 +34,28 @@
> * SUCH DAMAGE.
> */
>
> -#include <sys/param.h>
> +/* For __sym_compat(). */
> +#include <sys/cdefs.h>
>
> #include <errno.h>
> #include <stdlib.h>
> #include <unistd.h>
>
> -int
> -initgroups(const char *uname, gid_t agroup)
> +/* For freebsd14_setgroups(). */
> +#include "gen-compat.h"
> +
> +static int
> +initgroups_impl(const char *uname, gid_t agroup,
> + int (*setgroups)(int, const gid_t *))
> {
> - int ngroups, ret;
> - long ngroups_max;
> gid_t *groups;
> + long ngroups_max;
> + int ngroups, ret;
>
> /*
> - * Provide space for one group more than possible to allow
> - * setgroups to fail and set errno.
> + * Provide space for one group more than possible to allow setgroups()
> + * to fail and set 'errno' in case we get back more than {NGROUPS_MAX} +
> + * 1 groups.
> */
> ngroups_max = sysconf(_SC_NGROUPS_MAX) + 2;
> groups = malloc(sizeof(*groups) * ngroups_max);
> @@ -52,8 +63,23 @@ initgroups(const char *uname, gid_t agroup)
> return (-1); /* malloc() set 'errno'. */
>
> ngroups = (int)ngroups_max;
> - getgrouplist(uname, agroup, groups, &ngroups);
> - ret = setgroups(ngroups, groups);
> + (void)getgrouplist(uname, agroup, groups, &ngroups);
> + ret = (*setgroups)(ngroups, groups);
> +
> free(groups);
> return (ret); /* setgroups() set 'errno'. */
> }
> +
> +int
> +initgroups(const char *uname, gid_t agroup)
> +{
> + return (initgroups_impl(uname, agroup, setgroups));
> +}
> +
> +int
> +freebsd14_initgroups(const char *uname, gid_t agroup)
> +{
> + return (initgroups_impl(uname, agroup, freebsd14_setgroups));
> +}
> +
> +__sym_compat(initgroups, freebsd14_initgroups, FBSD_1.0);