Re: CFT: final patches for NGROUPS>>16

As usual, I forgot the attachments.  Here they are.

-- Brooks

On Wed, Jun 17, 2009 at 07:04:07PM -0500, Brooks Davis wrote:
> 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