From 54404cfb13d40e474f72d3c75de1f4db3417df64 Mon Sep 17 00:00:00 2001 From: Brooks Davis Date: Fri, 19 Jun 2009 15:58:24 +0000 Subject: [PATCH] In preparation for raising NGROUPS and NGROUPS_MAX, change base system callers of getgroups(), getgrouplist(), and setgroups() to allocate buffers dynamically. Specifically, allocate a buffer of size sysconf(_SC_NGROUPS_MAX)+1 (+2 in a few cases to allow for overflow). This (or similar gymnastics) is required for the code to actually follow the POSIX.1-2008 specification where {NGROUPS_MAX} may differ at runtime and where getgroups may return {NGROUPS_MAX}+1 results on systems like FreeBSD which include the primary group. In id(1), don't pointlessly add the primary group to the list of all groups, it is always the first result from getgroups(). In principle the old code was more portable, but this was only done in one of the two places where getgroups() was called to the overall effect was pointless. Document the actual POSIX requirements in the getgroups(2) and setgroups(2) manpages. We do not yet support a dynamic NGROUPS, but we may in the future. MFC after: 2 weeks --- lib/libc/gen/initgroups.3 | 7 +++++++ lib/libc/gen/initgroups.c | 21 +++++++++++++++------ lib/libc/rpc/auth_unix.c | 18 ++++++++++++------ lib/libc/sys/getgroups.2 | 10 ++++++---- lib/libc/sys/setgroups.2 | 6 ++---- usr.bin/id/id.c | 25 ++++++++++++++++++------- usr.bin/newgrp/newgrp.c | 12 ++++++++---- usr.bin/quota/quota.c | 10 ++++++++-- usr.sbin/chown/chown.c | 8 ++++++-- usr.sbin/chroot/chroot.c | 10 +++++++--- usr.sbin/jail/jail.c | 9 +++++++-- usr.sbin/jexec/jexec.c | 9 +++++++-- usr.sbin/lpr/lpc/lpc.c | 8 ++++++-- 13 files changed, 109 insertions(+), 44 deletions(-) diff --git a/lib/libc/gen/initgroups.3 b/lib/libc/gen/initgroups.3 index 2fe2dd984e16..3ed3ca76aae4 100644 --- a/lib/libc/gen/initgroups.3 +++ b/lib/libc/gen/initgroups.3 @@ -65,6 +65,13 @@ function may fail and set .Va errno for any of the errors specified for the library function .Xr setgroups 2 . +It may also return: +.Bl -tag -width Er +.It Bq Er ENOMEM +The +.Fn initgroups +function was unable to allocate temporary storage. +.El .Sh SEE ALSO .Xr setgroups 2 , .Xr getgrouplist 3 diff --git a/lib/libc/gen/initgroups.c b/lib/libc/gen/initgroups.c index 299ae503556c..aacaf7e647d0 100644 --- a/lib/libc/gen/initgroups.c +++ b/lib/libc/gen/initgroups.c @@ -35,10 +35,12 @@ __FBSDID("$FreeBSD$"); #include -#include #include "namespace.h" #include #include "un-namespace.h" +#include +#include +#include #include int @@ -46,14 +48,21 @@ initgroups(uname, agroup) const char *uname; gid_t agroup; { - int ngroups; + int ngroups, ret; + long ngroups_max; + gid_t *groups; + /* - * Provide space for one group more than NGROUPS to allow + * Provide space for one group more than possible to allow * setgroups to fail and set errno. */ - gid_t groups[NGROUPS + 1]; + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 2; + if ((groups = malloc(sizeof(*groups) * ngroups_max)) == NULL) + return (ENOMEM); - ngroups = NGROUPS + 1; + ngroups = (int)ngroups_max; getgrouplist(uname, agroup, groups, &ngroups); - return (setgroups(ngroups, groups)); + ret = setgroups(ngroups, groups); + free(groups); + return (ret); } diff --git a/lib/libc/rpc/auth_unix.c b/lib/libc/rpc/auth_unix.c index 5801015c25d1..ff3ca7b675c0 100644 --- a/lib/libc/rpc/auth_unix.c +++ b/lib/libc/rpc/auth_unix.c @@ -185,23 +185,29 @@ authunix_create(machname, uid, gid, len, aup_gids) AUTH * authunix_create_default() { - int len; + int ngids; + long ngids_max; char machname[MAXHOSTNAMELEN + 1]; uid_t uid; gid_t gid; - gid_t gids[NGROUPS_MAX]; + gid_t *gids; + + ngids_max = sysconf(_SC_NGROUPS_MAX) + 1; + gids = malloc(sizeof(gid_t) * ngids_max); + if (gids == NULL) + return (NULL); if (gethostname(machname, sizeof machname) == -1) abort(); machname[sizeof(machname) - 1] = 0; uid = geteuid(); gid = getegid(); - if ((len = getgroups(NGROUPS_MAX, gids)) < 0) + if ((ngids = getgroups(ngids_max, gids)) < 0) abort(); - if (len > NGRPS) - len = NGRPS; + if (ngids > NGRPS) + ngids = NGRPS; /* XXX: interface problem; those should all have been unsigned */ - return (authunix_create(machname, (int)uid, (int)gid, len, + return (authunix_create(machname, (int)uid, (int)gid, ngids, (int *)gids)); } diff --git a/lib/libc/sys/getgroups.2 b/lib/libc/sys/getgroups.2 index 5eadfe18b0ec..4fd8fee568a1 100644 --- a/lib/libc/sys/getgroups.2 +++ b/lib/libc/sys/getgroups.2 @@ -58,10 +58,7 @@ The system call returns the actual number of groups returned in .Fa gidset . -No more than -.Dv NGROUPS_MAX -will ever -be returned. +At least one and as many as {NGROUPS_MAX}+1 values may be returned. If .Fa gidsetlen is zero, @@ -92,6 +89,11 @@ an invalid address. .Sh SEE ALSO .Xr setgroups 2 , .Xr initgroups 3 +.Sh STANDARDS +The +.Fn getgroups +system call conforms to +.St -p1003.1-2008 . .Sh HISTORY The .Fn getgroups diff --git a/lib/libc/sys/setgroups.2 b/lib/libc/sys/setgroups.2 index e56e95f7f754..ef4c34c266e1 100644 --- a/lib/libc/sys/setgroups.2 +++ b/lib/libc/sys/setgroups.2 @@ -53,9 +53,7 @@ The argument indicates the number of entries in the array and must be no more than -.Dv NGROUPS , -as defined in -.In sys/param.h . +.Dv {NGROUPS_MAX}+1 . .Pp Only the super-user may set a new group list. .Sh RETURN VALUES @@ -71,7 +69,7 @@ The caller is not the super-user. The number specified in the .Fa ngroups argument is larger than the -.Dv NGROUPS +.Dv {NGROUPS_MAX}+1 limit. .It Bq Er EFAULT The address specified for diff --git a/usr.bin/id/id.c b/usr.bin/id/id.c index d18e8b0ea788..1929d968dd1d 100644 --- a/usr.bin/id/id.c +++ b/usr.bin/id/id.c @@ -258,7 +258,8 @@ id_print(struct passwd *pw, int use_ggl, int p_euid, int p_egid) gid_t gid, egid, lastgid; uid_t uid, euid; int cnt, ngroups; - gid_t groups[NGROUPS + 1]; + long ngroups_max; + gid_t *groups; const char *fmt; if (pw != NULL) { @@ -270,12 +271,16 @@ id_print(struct passwd *pw, int use_ggl, int p_euid, int p_egid) gid = getgid(); } + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((groups = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); + if (use_ggl && pw != NULL) { - ngroups = NGROUPS + 1; + ngroups = ngroups_max; getgrouplist(pw->pw_name, gid, groups, &ngroups); } else { - ngroups = getgroups(NGROUPS + 1, groups); + ngroups = getgroups(ngroups_max, groups); } if (pw != NULL) @@ -306,6 +311,7 @@ id_print(struct passwd *pw, int use_ggl, int p_euid, int p_egid) lastgid = gid; } printf("\n"); + free(groups); } #ifdef USE_BSM_AUDIT @@ -361,15 +367,19 @@ group(struct passwd *pw, int nflag) { struct group *gr; int cnt, id, lastid, ngroups; - gid_t groups[NGROUPS + 1]; + long ngroups_max; + gid_t *groups; const char *fmt; + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((groups = malloc(sizeof(gid_t) * (ngroups_max))) == NULL) + err(1, "malloc"); + if (pw) { - ngroups = NGROUPS + 1; + ngroups = ngroups_max; (void) getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups); } else { - groups[0] = getgid(); - ngroups = getgroups(NGROUPS, groups + 1) + 1; + ngroups = getgroups(ngroups_max, groups); } fmt = nflag ? "%s" : "%u"; for (lastid = -1, cnt = 0; cnt < ngroups; ++cnt) { @@ -389,6 +399,7 @@ group(struct passwd *pw, int nflag) lastid = id; } (void)printf("\n"); + free(groups); } void diff --git a/usr.bin/newgrp/newgrp.c b/usr.bin/newgrp/newgrp.c index 62d552fa05d3..91b62a52779d 100644 --- a/usr.bin/newgrp/newgrp.c +++ b/usr.bin/newgrp/newgrp.c @@ -146,8 +146,8 @@ restoregrps(void) static void addgroup(const char *grpname) { - gid_t grps[NGROUPS_MAX]; - long lgid; + gid_t *grps; + long lgid, ngrps_max; int dbmember, i, ngrps; gid_t egid; struct group *grp; @@ -185,7 +185,10 @@ addgroup(const char *grpname) } } - if ((ngrps = getgroups(NGROUPS_MAX, (gid_t *)grps)) < 0) { + ngrps_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((grps = malloc(sizeof(gid_t) * ngrps_max)) == NULL) + err(1, "malloc"); + if ((ngrps = getgroups(ngrps_max, (gid_t *)grps)) < 0) { warn("getgroups"); return; } @@ -217,7 +220,7 @@ addgroup(const char *grpname) /* Add old effective gid to supp. list if it does not exist. */ if (egid != grp->gr_gid && !inarray(egid, grps, ngrps)) { - if (ngrps == NGROUPS_MAX) + if (ngrps == ngrps_max) warnx("too many groups"); else { grps[ngrps++] = egid; @@ -231,6 +234,7 @@ addgroup(const char *grpname) } } + free(grps); } static int diff --git a/usr.bin/quota/quota.c b/usr.bin/quota/quota.c index 503b43d5233f..f504e7c31593 100644 --- a/usr.bin/quota/quota.c +++ b/usr.bin/quota/quota.c @@ -117,7 +117,8 @@ int main(int argc, char *argv[]) { int ngroups; - gid_t mygid, gidset[NGROUPS]; + long ngroups_max; + gid_t mygid, *gidset; int i, ch, gflag = 0, uflag = 0, errflag = 0; while ((ch = getopt(argc, argv, "f:ghlrquv")) != -1) { @@ -159,13 +160,18 @@ main(int argc, char *argv[]) errflag += showuid(getuid()); if (gflag) { mygid = getgid(); - ngroups = getgroups(NGROUPS, gidset); + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((gidset = malloc(sizeof(gid_t) * ngroups_max)) + == NULL) + err(1, "malloc"); + ngroups = getgroups(ngroups_max, gidset); if (ngroups < 0) err(1, "getgroups"); errflag += showgid(mygid); for (i = 0; i < ngroups; i++) if (gidset[i] != mygid) errflag += showgid(gidset[i]); + free(gidset); } return(errflag); } diff --git a/usr.sbin/chown/chown.c b/usr.sbin/chown/chown.c index 0918265840e5..b79decaa6a08 100644 --- a/usr.sbin/chown/chown.c +++ b/usr.sbin/chown/chown.c @@ -269,7 +269,8 @@ chownerr(const char *file) { static uid_t euid = -1; static int ngroups = -1; - gid_t groups[NGROUPS_MAX]; + static long ngroups_max; + gid_t *groups; /* Check for chown without being root. */ if (errno != EPERM || (uid != (uid_t)-1 && @@ -281,7 +282,10 @@ chownerr(const char *file) /* Check group membership; kernel just returns EPERM. */ if (gid != (gid_t)-1 && ngroups == -1 && euid == (uid_t)-1 && (euid = geteuid()) != 0) { - ngroups = getgroups(NGROUPS_MAX, groups); + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((groups = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); + ngroups = getgroups(ngroups_max, groups); while (--ngroups >= 0 && gid != groups[ngroups]); if (ngroups < 0) { warnx("you are not a member of group %s", gname); diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c index f33db7a24588..cc924a027bc5 100644 --- a/usr.sbin/chroot/chroot.c +++ b/usr.sbin/chroot/chroot.c @@ -69,9 +69,10 @@ main(argc, argv) struct passwd *pw; char *endp, *p; const char *shell; - gid_t gid, gidlist[NGROUPS_MAX]; + gid_t gid, *gidlist; uid_t uid; int ch, gids; + long ngroups_max; gid = 0; uid = 0; @@ -117,8 +118,11 @@ main(argc, argv) } } + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); for (gids = 0; - (p = strsep(&grouplist, ",")) != NULL && gids < NGROUPS_MAX; ) { + (p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) { if (*p == '\0') continue; @@ -135,7 +139,7 @@ main(argc, argv) } gids++; } - if (p != NULL && gids == NGROUPS_MAX) + if (p != NULL && gids == ngroups_max) errx(1, "too many supplementary groups provided"); if (user != NULL) { diff --git a/usr.sbin/jail/jail.c b/usr.sbin/jail/jail.c index 3963de59b175..e00610bf951a 100644 --- a/usr.sbin/jail/jail.c +++ b/usr.sbin/jail/jail.c @@ -104,7 +104,7 @@ extern char **environ; lcap = login_getpwclass(pwd); \ if (lcap == NULL) \ err(1, "getpwclass: %s", username); \ - ngroups = NGROUPS; \ + ngroups = ngroups_max; \ if (getgrouplist(username, pwd->pw_gid, groups, &ngroups) != 0) \ err(1, "getgrouplist: %s", username); \ } while (0) @@ -115,10 +115,11 @@ main(int argc, char **argv) login_cap_t *lcap = NULL; struct iovec rparams[2]; struct passwd *pwd = NULL; - gid_t groups[NGROUPS]; + gid_t *groups; size_t sysvallen; int ch, cmdarg, i, jail_set_flags, jid, ngroups, sysval; int hflag, iflag, Jflag, lflag, rflag, uflag, Uflag; + long ngroups_max; unsigned pi; char *ep, *jailname, *securelevel, *username, *JidFile; char errmsg[ERRMSG_SIZE], enforce_statfs[4]; @@ -132,6 +133,10 @@ main(int argc, char **argv) jailname = securelevel = username = JidFile = cleanenv = NULL; fp = NULL; + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((groups = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); + while ((ch = getopt(argc, argv, "cdhilmn:r:s:u:U:J:")) != -1) { switch (ch) { case 'd': diff --git a/usr.sbin/jexec/jexec.c b/usr.sbin/jexec/jexec.c index e86657bf5f67..143bbd5acb59 100644 --- a/usr.sbin/jexec/jexec.c +++ b/usr.sbin/jexec/jexec.c @@ -59,7 +59,7 @@ static void usage(void); lcap = login_getpwclass(pwd); \ if (lcap == NULL) \ err(1, "getpwclass: %s", username); \ - ngroups = NGROUPS; \ + ngroups = ngroups_max; \ if (getgrouplist(username, pwd->pw_gid, groups, &ngroups) != 0) \ err(1, "getgrouplist: %s", username); \ } while (0) @@ -71,12 +71,17 @@ main(int argc, char *argv[]) int jid; login_cap_t *lcap = NULL; struct passwd *pwd = NULL; - gid_t groups[NGROUPS]; + gid_t *groups = NULL; int ch, ngroups, uflag, Uflag; + long ngroups_max; char *ep, *username; ch = uflag = Uflag = 0; username = NULL; + ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1; + if ((groups = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); + while ((ch = getopt(argc, argv, "nu:U:")) != -1) { switch (ch) { case 'n': diff --git a/usr.sbin/lpr/lpc/lpc.c b/usr.sbin/lpr/lpc/lpc.c index e279f3ab5a08..fda77aa30c24 100644 --- a/usr.sbin/lpr/lpc/lpc.c +++ b/usr.sbin/lpr/lpc/lpc.c @@ -356,7 +356,8 @@ ingroup(const char *grname) { static struct group *gptr=NULL; static int ngroups = 0; - static gid_t groups[NGROUPS]; + static long ngroups_max; + static gid_t *groups; register gid_t gid; register int i; @@ -365,7 +366,10 @@ ingroup(const char *grname) warnx("warning: unknown group '%s'", grname); return(0); } - ngroups = getgroups(NGROUPS, groups); + ngroups_max = sysconf(_SC_NGROUPS_MAX); + if ((groups = malloc(sizeof(gid_t) * ngroups_max)) == NULL) + err(1, "malloc"); + ngroups = getgroups(ngroups_max, groups); if (ngroups < 0) err(1, "getgroups"); }