diff --git a/share/man/man9/ucred.9 b/share/man/man9/ucred.9 --- a/share/man/man9/ucred.9 +++ b/share/man/man9/ucred.9 @@ -24,7 +24,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH .\" DAMAGE. .\" -.Dd January 23, 2019 +.Dd July 29, 2025 .Dt UCRED 9 .Os .Sh NAME @@ -119,8 +119,7 @@ groups. No other mechanism should be used to modify the .Va cr_groups -array except for updating the primary group via assignment to -.Va cr_groups[0] . +array. .Pp The .Fn cru2x diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c --- a/sys/compat/linux/linux_misc.c +++ b/sys/compat/linux/linux_misc.c @@ -1030,47 +1030,32 @@ { struct ucred *newcred, *oldcred; l_gid_t *linux_gidset; - gid_t *bsd_gidset; int ngrp, error; struct proc *p; ngrp = args->gidsetsize; - if (ngrp < 0 || ngrp >= ngroups_max + 1) + if (ngrp < 0 || ngrp >= ngroups_max) return (EINVAL); linux_gidset = malloc(ngrp * sizeof(*linux_gidset), M_LINUX, M_WAITOK); error = copyin(args->grouplist, linux_gidset, ngrp * sizeof(l_gid_t)); if (error) goto out; newcred = crget(); - crextend(newcred, ngrp + 1); + crextend(newcred, ngrp); p = td->td_proc; PROC_LOCK(p); oldcred = p->p_ucred; crcopy(newcred, oldcred); - /* - * cr_groups[0] holds egid. Setting the whole set from - * the supplied set will cause egid to be changed too. - * Keep cr_groups[0] unchanged to prevent that. - */ - if ((error = priv_check_cred(oldcred, PRIV_CRED_SETGROUPS)) != 0) { PROC_UNLOCK(p); crfree(newcred); goto out; } - if (ngrp > 0) { - newcred->cr_ngroups = ngrp + 1; - - bsd_gidset = newcred->cr_groups; - ngrp--; - while (ngrp >= 0) { - bsd_gidset[ngrp + 1] = linux_gidset[ngrp]; - ngrp--; - } - } else - newcred->cr_ngroups = 1; + newcred->cr_ngroups = ngrp; + for (int i = 0; i < ngrp; i++) + newcred->cr_groups[i] = linux_gidset[i]; setsugid(p); proc_set_cred(p, newcred); @@ -1092,13 +1077,7 @@ cred = td->td_ucred; bsd_gidset = cred->cr_groups; - bsd_gidsetsz = cred->cr_ngroups - 1; - - /* - * cr_groups[0] holds egid. Returning the whole set - * here will cause a duplicate. Exclude cr_groups[0] - * to prevent that. - */ + bsd_gidsetsz = cred->cr_ngroups; if ((ngrp = args->gidsetsize) == 0) { td->td_retval[0] = bsd_gidsetsz; @@ -1112,7 +1091,7 @@ linux_gidset = malloc(bsd_gidsetsz * sizeof(*linux_gidset), M_LINUX, M_WAITOK); while (ngrp < bsd_gidsetsz) { - linux_gidset[ngrp] = bsd_gidset[ngrp + 1]; + linux_gidset[ngrp] = bsd_gidset[ngrp]; ngrp++; } diff --git a/sys/compat/linux/linux_uid16.c b/sys/compat/linux/linux_uid16.c --- a/sys/compat/linux/linux_uid16.c +++ b/sys/compat/linux/linux_uid16.c @@ -87,12 +87,11 @@ { struct ucred *newcred, *oldcred; l_gid16_t *linux_gidset; - gid_t *bsd_gidset; int ngrp, error; struct proc *p; ngrp = args->gidsetsize; - if (ngrp < 0 || ngrp >= ngroups_max + 1) + if (ngrp < 0 || ngrp >= ngroups_max) return (EINVAL); linux_gidset = malloc(ngrp * sizeof(*linux_gidset), M_LINUX, M_WAITOK); error = copyin(args->gidset, linux_gidset, ngrp * sizeof(l_gid16_t)); @@ -106,12 +105,6 @@ PROC_LOCK(p); oldcred = crcopysafe(p, newcred); - /* - * cr_groups[0] holds egid. Setting the whole set from - * the supplied set will cause egid to be changed too. - * Keep cr_groups[0] unchanged to prevent that. - */ - if ((error = priv_check_cred(oldcred, PRIV_CRED_SETGROUPS)) != 0) { PROC_UNLOCK(p); crfree(newcred); @@ -121,18 +114,9 @@ goto out; } - if (ngrp > 0) { - newcred->cr_ngroups = ngrp + 1; - - bsd_gidset = newcred->cr_groups; - ngrp--; - while (ngrp >= 0) { - bsd_gidset[ngrp + 1] = linux_gidset[ngrp]; - ngrp--; - } - } - else - newcred->cr_ngroups = 1; + newcred->cr_ngroups = ngrp; + for (int i = 0; i < ngrp; i++) + newcred->cr_groups[i] = linux_gidset[i]; setsugid(td->td_proc); proc_set_cred(p, newcred); @@ -155,13 +139,7 @@ cred = td->td_ucred; bsd_gidset = cred->cr_groups; - bsd_gidsetsz = cred->cr_ngroups - 1; - - /* - * cr_groups[0] holds egid. Returning the whole set - * here will cause a duplicate. Exclude cr_groups[0] - * to prevent that. - */ + bsd_gidsetsz = cred->cr_ngroups; if ((ngrp = args->gidsetsize) == 0) { td->td_retval[0] = bsd_gidsetsz; @@ -175,7 +153,7 @@ linux_gidset = malloc(bsd_gidsetsz * sizeof(*linux_gidset), M_LINUX, M_WAITOK); while (ngrp < bsd_gidsetsz) { - linux_gidset[ngrp] = bsd_gidset[ngrp + 1]; + linux_gidset[ngrp] = bsd_gidset[ngrp]; ngrp++; } diff --git a/sys/fs/nfs/nfs_commonport.c b/sys/fs/nfs/nfs_commonport.c --- a/sys/fs/nfs/nfs_commonport.c +++ b/sys/fs/nfs/nfs_commonport.c @@ -380,8 +380,7 @@ cred->cr_uid = 0; cred->cr_gid = 0; - /* XXXKE Fix this if cr_gid gets separated out. */ - cred->cr_ngroups = 1; + cred->cr_ngroups = 0; } /* diff --git a/sys/fs/nfsclient/nfs_clrpcops.c b/sys/fs/nfsclient/nfs_clrpcops.c --- a/sys/fs/nfsclient/nfs_clrpcops.c +++ b/sys/fs/nfsclient/nfs_clrpcops.c @@ -6934,8 +6934,7 @@ tcred = NFSNEWCRED(cred); tcred->cr_uid = flp->nfsfl_ffm[mirror].user; tcred->cr_gid = flp->nfsfl_ffm[mirror].group; - /* XXXKE Fix this if cr_gid gets separated out. */ - tcred->cr_ngroups = 1; + tcred->cr_ngroups = 0; } else tcred = cred; if (rwflag == NFSV4OPEN_ACCESSREAD) diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -3463,6 +3463,7 @@ NFSVNO_EXPORTANON(exp) || (nd->nd_flag & ND_AUTHNONE) != 0) { nd->nd_cred->cr_uid = credanon->cr_uid; + nd->nd_cred->cr_gid = credanon->cr_gid; /* * 'credanon' is already a 'struct ucred' that was built * internally with calls to crsetgroups_fallback(), so diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c --- a/sys/kern/kern_prot.c +++ b/sys/kern/kern_prot.c @@ -99,12 +99,11 @@ groups_check_positive_len(int ngrp) { MPASS2(ngrp >= 0, "negative number of groups"); - MPASS2(ngrp != 0, "at least one group expected (effective GID)"); } static inline void groups_check_max_len(int ngrp) { - MPASS2(ngrp <= ngroups_max + 1, "too many groups"); + MPASS2(ngrp <= ngroups_max, "too many supplementary groups"); } static void groups_normalize(int *ngrp, gid_t *groups); @@ -321,10 +320,17 @@ sys_getgroups(struct thread *td, struct getgroups_args *uap) { struct ucred *cred; + gid_t *ugidset; int ngrp, error; cred = td->td_ucred; - ngrp = cred->cr_ngroups; + + /* + * cr_gid has been moved out of cr_groups, but we'll continue exporting + * the egid as groups[0] for the time being until we audit userland for + * any surprises. + */ + ngrp = cred->cr_ngroups + 1; if (uap->gidsetsize == 0) { error = 0; @@ -333,7 +339,14 @@ if (uap->gidsetsize < ngrp) return (EINVAL); - error = copyout(cred->cr_groups, uap->gidset, ngrp * sizeof(gid_t)); + ugidset = uap->gidset; + error = copyout(&cred->cr_gid, ugidset, sizeof(*ugidset)); + if (error != 0) + goto out; + + if (ngrp > 1) + error = copyout(cred->cr_groups, ugidset + 1, + (ngrp - 1) * sizeof(*ugidset)); out: td->td_retval[0] = ngrp; return (error); @@ -499,8 +512,8 @@ } /* - * Final storage for groups (including the effective GID) will be returned via - * 'groups'. '*groups' must be NULL on input, and if not equal to 'smallgroups' + * Final storage for supplementary groups will be returned via 'groups'. + * '*groups' must be NULL on input, and if not equal to 'smallgroups' * on output, must be freed (M_TEMP) *even if* an error is returned. */ static int @@ -525,15 +538,15 @@ * now, to avoid having to allocate and copy again the * supplementary groups. */ - *groups = wcred->sc_supp_groups_nb < CRED_SMALLGROUPS_NB ? - smallgroups : malloc((wcred->sc_supp_groups_nb + 1) * + *groups = wcred->sc_supp_groups_nb <= CRED_SMALLGROUPS_NB ? + smallgroups : malloc(wcred->sc_supp_groups_nb * sizeof(*groups), M_TEMP, M_WAITOK); - error = copyin(wcred->sc_supp_groups, *groups + 1, + error = copyin(wcred->sc_supp_groups, *groups, wcred->sc_supp_groups_nb * sizeof(*groups)); if (error != 0) return (error); - wcred->sc_supp_groups = *groups + 1; + wcred->sc_supp_groups = *groups; } else { wcred->sc_supp_groups_nb = 0; wcred->sc_supp_groups = NULL; @@ -652,9 +665,8 @@ * CAUTION: This function normalizes groups in 'wcred'. * * If 'preallocated_groups' is non-NULL, it must be an already allocated array - * of size 'wcred->sc_supp_groups_nb + 1', with the supplementary groups - * starting at index 1, and 'wcred->sc_supp_groups' then must point to the first - * supplementary group. + * of size 'wcred->sc_supp_groups_nb' containing the supplementary groups, and + * 'wcred->sc_supp_groups' then must point to it. */ int kern_setcred(struct thread *const td, const u_int flags, @@ -685,13 +697,14 @@ return (EINVAL); if (preallocated_groups != NULL) { groups = preallocated_groups; - MPASS(preallocated_groups + 1 == wcred->sc_supp_groups); + MPASS(preallocated_groups == wcred->sc_supp_groups); } else { - groups = wcred->sc_supp_groups_nb < CRED_SMALLGROUPS_NB ? - smallgroups : - malloc((wcred->sc_supp_groups_nb + 1) * - sizeof(*groups), M_TEMP, M_WAITOK); - memcpy(groups + 1, wcred->sc_supp_groups, + if (wcred->sc_supp_groups_nb <= CRED_SMALLGROUPS_NB) + groups = smallgroups; + else + groups = malloc(wcred->sc_supp_groups_nb * + sizeof(*groups), M_TEMP, M_WAITOK); + memcpy(groups, wcred->sc_supp_groups, wcred->sc_supp_groups_nb * sizeof(*groups)); } } @@ -726,16 +739,12 @@ if (flags & SETCREDF_SVGID) AUDIT_ARG_SGID(wcred->sc_svgid); if (flags & SETCREDF_SUPP_GROUPS) { - int ngrp = wcred->sc_supp_groups_nb; - /* * Output the raw supplementary groups array for better * traceability. */ - AUDIT_ARG_GROUPSET(groups + 1, ngrp); - ++ngrp; - groups_normalize(&ngrp, groups); - wcred->sc_supp_groups_nb = ngrp - 1; + AUDIT_ARG_GROUPSET(groups, wcred->sc_supp_groups_nb); + groups_normalize(&wcred->sc_supp_groups_nb, groups); } /* @@ -746,7 +755,7 @@ new_cred = crget(); to_free_cred = new_cred; if (flags & SETCREDF_SUPP_GROUPS) - crextend(new_cred, wcred->sc_supp_groups_nb + 1); + crextend(new_cred, wcred->sc_supp_groups_nb); #ifdef MAC mac_cred_setcred_enter(); @@ -773,16 +782,11 @@ /* * Change groups. - * - * crsetgroups_internal() changes both the effective and supplementary - * ones. */ - if (flags & SETCREDF_SUPP_GROUPS) { - groups[0] = flags & SETCREDF_GID ? wcred->sc_gid : - new_cred->cr_gid; - crsetgroups_internal(new_cred, wcred->sc_supp_groups_nb + 1, + if (flags & SETCREDF_SUPP_GROUPS) + crsetgroups_internal(new_cred, wcred->sc_supp_groups_nb, groups); - } else if (flags & SETCREDF_GID) + if (flags & SETCREDF_GID) change_egid(new_cred, wcred->sc_gid); if (flags & SETCREDF_RGID) change_rgid(new_cred, wcred->sc_rgid); @@ -1206,6 +1210,7 @@ * setgroups() differ. */ gidsetsize = uap->gidsetsize; + /* XXXKE Limit to ngroups_max when we change the userland interface. */ if (gidsetsize > ngroups_max + 1 || gidsetsize < 0) return (EINVAL); @@ -1233,29 +1238,49 @@ struct proc *p = td->td_proc; struct ucred *newcred, *oldcred; int ngrp, error; + gid_t egid; ngrp = *ngrpp; /* Sanity check size. */ + /* XXXKE Limit to ngroups_max when we change the userland interface. */ if (ngrp < 0 || ngrp > ngroups_max + 1) return (EINVAL); AUDIT_ARG_GROUPSET(groups, ngrp); + /* + * setgroups(0, NULL) is a legitimate way of clearing the groups vector + * on non-BSD systems (which generally do not have the egid in the + * groups[0]). We risk security holes when running non-BSD software if + * we do not do the same. So we allow and treat 0 for 'ngrp' specially + * below (twice). + */ if (ngrp != 0) { - /* We allow and treat 0 specially below. */ - groups_normalize(ngrpp, groups); - ngrp = *ngrpp; + /* + * To maintain userland compat for now, we use the first group + * as our egid and we'll use the rest as our supplemental + * groups. + */ + egid = groups[0]; + ngrp--; + groups++; + + groups_normalize(&ngrp, groups); + *ngrpp = ngrp; } newcred = crget(); - if (ngrp != 0) - crextend(newcred, ngrp); + crextend(newcred, ngrp); PROC_LOCK(p); oldcred = crcopysafe(p, newcred); #ifdef MAC - error = ngrp == 0 ? - /* If 'ngrp' is 0, we'll keep just the current effective GID. */ - mac_cred_check_setgroups(oldcred, 1, oldcred->cr_groups) : - mac_cred_check_setgroups(oldcred, ngrp, groups); + /* + * We pass NULL here explicitly if we don't have any supplementary + * groups mostly for the sake of normalization, but also to avoid/detect + * a situation where a MAC module has some assumption about the layout + * of `groups` matching historical behavior. + */ + error = mac_cred_check_setgroups(oldcred, ngrp, + ngrp == 0 ? NULL : groups); if (error) goto fail; #endif @@ -1264,16 +1289,14 @@ if (error) goto fail; - if (ngrp == 0) { - /* - * setgroups(0, NULL) is a legitimate way of clearing the - * groups vector on non-BSD systems (which generally do not - * have the egid in the groups[0]). We risk security holes - * when running non-BSD software if we do not do the same. - */ - newcred->cr_ngroups = 1; - } else - crsetgroups_internal(newcred, ngrp, groups); + /* + * If some groups were passed, the first one is currently the desired + * egid. This code is to be removed (along with some commented block + * above) when setgroups() is changed to take only supplementary groups. + */ + if (ngrp != 0) + newcred->cr_gid = egid; + crsetgroups_internal(newcred, ngrp, groups); setsugid(p); proc_set_cred(p, newcred); @@ -1693,11 +1716,11 @@ groups_check_positive_len(ngrp); groups_check_max_len(ngrp); - if (ngrp == 1) + if (ngrp <= 1) return; - prev_g = groups[1]; - for (int i = 2; i < ngrp; ++i) { + prev_g = groups[0]; + for (int i = 1; i < ngrp; ++i) { const gid_t g = groups[i]; if (prev_g >= g) @@ -1723,7 +1746,7 @@ * Perform a binary search of the supplementary groups. This is * possible because we sort the groups in crsetgroups(). */ - return (bsearch(&gid, cred->cr_groups + 1, cred->cr_ngroups - 1, + return (bsearch(&gid, cred->cr_groups, cred->cr_ngroups, sizeof(gid), gidp_cmp) != NULL); } @@ -2588,11 +2611,6 @@ crcopy(struct ucred *dest, struct ucred *src) { - /* - * Ideally, 'cr_ngroups' should be moved out of 'struct ucred''s bcopied - * area, but this would break the ABI, so is deferred until there is - * a compelling need to change it. - */ bcopy(&src->cr_startcopy, &dest->cr_startcopy, (unsigned)((caddr_t)&src->cr_endcopy - (caddr_t)&src->cr_startcopy)); @@ -2634,11 +2652,17 @@ bzero(xcr, sizeof(*xcr)); xcr->cr_version = XUCRED_VERSION; xcr->cr_uid = cr->cr_uid; + xcr->cr_gid = cr->cr_gid; - ngroups = MIN(cr->cr_ngroups, XU_NGROUPS); + /* + * We use a union to alias cr_gid to cr_groups[0] in the xucred, so + * this is kind of ugly; cr_ngroups still includes the egid for our + * purposes to avoid bumping the xucred version. + */ + ngroups = MIN(cr->cr_ngroups + 1, nitems(xcr->cr_groups)); xcr->cr_ngroups = ngroups; - bcopy(cr->cr_groups, xcr->cr_groups, - ngroups * sizeof(*cr->cr_groups)); + bcopy(cr->cr_groups, xcr->cr_sgroups, + (ngroups - 1) * sizeof(*cr->cr_groups)); } void @@ -2809,12 +2833,8 @@ /* * Normalizes a set of groups to be applied to a 'struct ucred'. * - * The set of groups is an array that must comprise the effective GID as its - * first element (so its length cannot be 0). - * - * Normalization ensures that elements after the first, which stand for the - * supplementary groups, are sorted in ascending order and do not contain - * duplicates. + * Normalization ensures that the supplementary groups are sorted in ascending + * order and do not contain duplicates. */ static void groups_normalize(int *ngrp, gid_t *groups) @@ -2825,15 +2845,15 @@ groups_check_positive_len(*ngrp); groups_check_max_len(*ngrp); - if (*ngrp == 1) + if (*ngrp <= 1) return; - qsort(groups + 1, *ngrp - 1, sizeof(*groups), gidp_cmp); + qsort(groups, *ngrp, sizeof(*groups), gidp_cmp); /* Remove duplicates. */ - prev_g = groups[1]; - ins_idx = 2; - for (int i = 2; i < *ngrp; ++i) { + prev_g = groups[0]; + ins_idx = 1; + for (int i = ins_idx; i < *ngrp; ++i) { const gid_t g = groups[i]; if (g != prev_g) { @@ -2876,7 +2896,7 @@ * Copy groups in to a credential after expanding it if required. * * May sleep in order to allocate memory (except if, e.g., crextend() was called - * before with 'ngrp' or greater). Truncates the list to (ngroups_max + 1) if + * before with 'ngrp' or greater). Truncates the list to ngroups_max if * it is too large. Array 'groups' doesn't need to be sorted. 'ngrp' must be * strictly positive. */ @@ -2884,8 +2904,8 @@ crsetgroups(struct ucred *cr, int ngrp, const gid_t *groups) { - if (ngrp > ngroups_max + 1) - ngrp = ngroups_max + 1; + if (ngrp > ngroups_max) + ngrp = ngroups_max; /* * crextend() asserts that groups are not set, as it may allocate a new * backing storage without copying the content of the old one. Since we @@ -2893,6 +2913,9 @@ * consider the old ones thrown away. */ cr->cr_ngroups = 0; + if (ngrp == 0) + return; + crextend(cr, ngrp); crsetgroups_internal(cr, ngrp, groups); groups_normalize(&cr->cr_ngroups, cr->cr_groups); @@ -2902,18 +2925,22 @@ * Same as crsetgroups() but accepts an empty groups array. * * This function ensures that an effective GID is always present in credentials. - * An empty array is treated as a one-size one holding the passed effective GID - * fallback. + * An empty array will only set the effective GID to the fallback, while a + * non-empty array will peel off groups[0] to set as the effective GID and use + * the remainder, if any, as supplementary groups. */ void crsetgroups_fallback(struct ucred *cr, int ngrp, const gid_t *groups, const gid_t fallback) { - if (ngrp == 0) - /* Shortcut. */ - crsetgroups_internal(cr, 1, &fallback); - else - crsetgroups(cr, ngrp, groups); + if (ngrp == 0) { + cr->cr_gid = fallback; + cr->cr_ngroups = 0; + return; + } + + crsetgroups(cr, ngrp - 1, groups + 1); + cr->cr_gid = groups[0]; } /* diff --git a/sys/rpc/authunix_prot.c b/sys/rpc/authunix_prot.c --- a/sys/rpc/authunix_prot.c +++ b/sys/rpc/authunix_prot.c @@ -96,8 +96,12 @@ if (!xdr_uint32_t(xdrs, &cred->cr_gid)) return (FALSE); - /* XXXKE Fix this is cr_gid gets separated out. */ if (xdrs->x_op == XDR_ENCODE) { + /* + * Note that this is a `struct xucred`, which maintains its + * historical layout of preserving the egid in cr_ngroups and + * cr_groups[0] == egid. + */ ngroups = cred->cr_ngroups - 1; if (ngroups > NGRPS) ngroups = NGRPS; diff --git a/sys/rpc/svc_auth.c b/sys/rpc/svc_auth.c --- a/sys/rpc/svc_auth.c +++ b/sys/rpc/svc_auth.c @@ -39,6 +39,7 @@ */ #include +#include #include #include #include @@ -191,7 +192,7 @@ return (FALSE); cr = crget(); cr->cr_uid = cr->cr_ruid = cr->cr_svuid = xprt->xp_uid; - crsetgroups(cr, xprt->xp_ngrps, xprt->xp_gidp); + crsetgroups_fallback(cr, xprt->xp_ngrps, xprt->xp_gidp, GID_NOGROUP); cr->cr_rgid = cr->cr_svgid = cr->cr_gid; cr->cr_prison = curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); @@ -206,7 +207,7 @@ return (FALSE); cr = crget(); cr->cr_uid = cr->cr_ruid = cr->cr_svuid = xcr->cr_uid; - crsetgroups(cr, xcr->cr_ngroups, xcr->cr_groups); + crsetgroups_fallback(cr, xcr->cr_ngroups, xcr->cr_groups, GID_NOGROUP); cr->cr_rgid = cr->cr_svgid = cr->cr_gid; cr->cr_prison = curthread->td_ucred->cr_prison; prison_hold(cr->cr_prison); diff --git a/sys/rpc/svc_auth_unix.c b/sys/rpc/svc_auth_unix.c --- a/sys/rpc/svc_auth_unix.c +++ b/sys/rpc/svc_auth_unix.c @@ -89,8 +89,12 @@ stat = AUTH_BADCRED; goto done; } - /* XXXKE Fix this if cr_gid gets separated out. */ for (i = 0; i < gid_len; i++) { + /* + * Note that this is a `struct xucred`, which maintains + * its historical layout of preserving the egid in + * cr_ngroups and cr_groups[0] == egid. + */ if (i + 1 < XU_NGROUPS) xcr->cr_groups[i + 1] = IXDR_GET_INT32(buf); else diff --git a/sys/sys/ucred.h b/sys/sys/ucred.h --- a/sys/sys/ucred.h +++ b/sys/sys/ucred.h @@ -76,15 +76,12 @@ u_int cr_users; /* (c) proc + thread using this cred */ u_int cr_flags; /* credential flags */ struct auditinfo_addr cr_audit; /* Audit properties. */ + int cr_ngroups; /* number of supplementary groups */ #define cr_startcopy cr_uid uid_t cr_uid; /* effective user id */ uid_t cr_ruid; /* real user id */ uid_t cr_svuid; /* saved user id */ - /* - * XXXOC: On the next ABI change, please move 'cr_ngroups' out of the - * copied area (crcopy() already copes with this change). - */ - int cr_ngroups; /* number of groups */ + gid_t cr_gid; /* effective group id */ gid_t cr_rgid; /* real group id */ gid_t cr_svgid; /* saved group id */ struct uidinfo *cr_uidinfo; /* per euid resource consumption */ @@ -111,8 +108,20 @@ struct xucred { u_int cr_version; /* structure layout version */ uid_t cr_uid; /* effective user id */ - short cr_ngroups; /* number of groups */ - gid_t cr_groups[XU_NGROUPS]; /* groups */ + short cr_ngroups; /* number of groups (incl. cr_gid). */ + union { + /* + * Special little hack to avoid needing a cr_gid macro, which + * would cause problems if one were to use it with struct ucred + * which also has a cr_groups member. + */ + struct { + gid_t cr_gid; /* effective group id */ + gid_t cr_sgroups[XU_NGROUPS - 1]; + }; + + gid_t cr_groups[XU_NGROUPS]; /* groups */ + }; union { void *_cr_unused1; /* compatibility with old ucred */ pid_t cr_pid; @@ -120,9 +129,6 @@ }; #define XUCRED_VERSION 0 -/* This can be used for both ucred and xucred structures. */ -#define cr_gid cr_groups[0] - struct mac; /* * Structure to pass as an argument to the setcred() system call. diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c --- a/sys/ufs/ufs/ufs_vnops.c +++ b/sys/ufs/ufs/ufs_vnops.c @@ -2038,7 +2038,6 @@ { #ifdef QUOTA struct ucred ucred, *ucp; - gid_t ucred_group; ucp = cnp->cn_cred; #endif /* @@ -2065,13 +2064,8 @@ */ ucred.cr_ref = 1; ucred.cr_uid = ip->i_uid; - - /* - * XXXKE Fix this is cr_gid gets separated out - */ - ucred.cr_ngroups = 1; - ucred.cr_groups = &ucred_group; - ucred.cr_gid = ucred_group = dp->i_gid; + ucred.cr_gid = dp->i_gid; + ucred.cr_ngroups = 0; ucp = &ucred; } #endif @@ -2802,7 +2796,6 @@ { #ifdef QUOTA struct ucred ucred, *ucp; - gid_t ucred_group; ucp = cnp->cn_cred; #endif /* @@ -2828,13 +2821,8 @@ */ ucred.cr_ref = 1; ucred.cr_uid = ip->i_uid; - - /* - * XXXKE Fix this is cr_gid gets separated out - */ - ucred.cr_ngroups = 1; - ucred.cr_groups = &ucred_group; - ucred.cr_gid = ucred_group = pdir->i_gid; + ucred.cr_gid = pdir->i_gid; + ucred.cr_ngroups = 0; ucp = &ucred; #endif } else {