Please find attached three patches which result in raising NGROUPS to 1024, making programs in the base system immune to changing values of NGROUPS, and pave the way for NGROUPS to be boot time configurable. The first two patches (ngroups-catman.diff and ngroups-posix.diff) are userland cleanups. The third (ngroups-main.diff) is the core of the change and primairly in the kernel. It is strongly based on work contributed by Isilon Systems. The proposed commit messages appear to the bottom of this message. I've been running the basic code on my FreeBSD laptop for a while and I've tested basic NFS mounts, but more areas need testing before this hits a release. Key things to hit include: - NFS mount and export - IPFW uid, gid, and jail rules - portalfs (ideally portalfs should be extended to not truncate groups) In practice, expect appliations to not care about this change unless they are run with more than 16 groups. If that happens, calls to getgroups() will fail if they use statically sized buffers recompilation should fix them. Please test, review, etc. I'd like to get this in before code freeze if at all possible. -- Brooks ngroups-catman.diff: When checking if we can write to a file, use access() instead of a manual permission check based on stat output. Also, get rid of the executability check since it is not used. MFC after: 2 weeks ngroups-posix.diff 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 ngroups-main.diff: Rework the credential code to support larger values of NGROUPS and NGROUPS_MAX, eliminate ABI dependencies on them, and raise the to 1024 and 1023 respectively. (Previously they were equal, but under a close reading of POSIX, NGROUPS_MAX was defined to be too large by 1 since it is the number of supplemental groups, not total number of groups.) The bulk of the change consists of converting the struct ucred member cr_groups from a static array to a pointer. Do the equivalent in kinfo_proc. Introduce new interfaces crcopysafe() and crsetgroups() for duplicating a process credential before modifying it and for setting group lists respectively. Both interfaces take care for the details of allocating groups array. crsetgroups() takes care of truncating the group list to the current maximum (NGROUPS) if necessary. In the future, crsetgroups() may be responsible for insuring invariants such as sorting the supplemental groups to allow groupmember() to be implemented as a binary search. Because we can not change struct xucred without breaking application ABIs, we leave it alone and introduce a new XU_NGROUPS value which is always 16 and is to be used or NGRPS as appropriate for things such as NFS which need to use no more than 16 groups. When feasible, truncate the group list rather than generating an error. Minor changes: - Reduce the number of hand rolled versions of groupmember(). - Do not assign to both cr_gid and cr_groups[0]. - Modify ipfw to cache ucreds instead of part of their contents since they are immutable once referenced by more than one entity. Submitted by: Isilon Systems X-MFC after: never
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:50 UTC