On Wed, Jun 27, 2001 at 06:40:53PM +1000, Bruce Evans wrote:
> On Wed, 27 Jun 2001, Ruslan Ermilov wrote:
> > On Tue, Jun 26, 2001 at 11:18:56AM -0400, Robert Watson wrote:
> > > ...
> > > off.  I'm generally fairly positive about this change, but would be
> > > interested in hearing Bruce's thoughts on any compatibility issues, in
> > > particular, with respects to the behavior of userland processes with
> > > expectations about the old behavior.  Obviously, this is a change that is
> Me too :-).  I don't know much about this except that it is related to
> longstanding bugs in gid management.
> > At least one compatibility issue here is that it's no longer possible
> > to use initgroups(3) to set the effective group ID.
> I think this shows that keeping the egid in group lists is intentional.
It's hard to say actually.  4.3BSD up to Tahoe and Net/1 had (in user.h)
explicit holder for egid and NGROUPS supplementary group IDs.

> The only bug in the current implementation seems to be that NGROUPS_MAX
> is 1 too small.  The first gid in group lists is conventionally always
> the egid, but there must be space for NGROUPS_MAX "supplementary" groups,
> so statically allocated group lists must have size NGROUPS_MAX+1, but
> they currently (all?) have size NGROUPS_MAX.  POSIX.1-200x documents this
> for getgroups(2) -- returning the egid is optional, and getgroups() may
> return {NGROUPS_MAX}+1 entries.
What's wrong with keeping cr_gid in a separate structure member?
Continuing to keep it inside the cr_groups[] would cause us to
deal with NGROUPS_MAX vs. NGROUPS_MAX + 1 calculations all over
the place inside the kernel.  This IMHO only unnecessary complicates
the things.

We could still preserve the old behavior of getgroups(2) returning
the effective GID, but this only makes sense if we also preserve
the semantics of setgroups(2) setting the effective GID, which is
bogus; setgroups(2) should only be allowed to set the supplementary
group IDs, like most other OSes do, including NetBSD since 1995.

> I think the semantics of getgroups(), setgroups() and initgroups()
> shouldn't be changed.
This isn't possible, as if we continue to return egid with getgroups(),
it will now return maximum {NGROUPS_MAX} + 1 gids, as opposed to the
currently documented "no more than NGROUPS_MAX will ever be returned",
thus breaking backwards compatibility anyway.

> To set a really supplemental gid (one not affected by setegid(),
> setgroups() must put the gid in the list after the first entry
> even if it is is the egid).
> In the kernel, the problem is not really changed by keeping the egid
> in a separate variable.  I currently slightly prefer keeping it in
> group lists.
Again, why?

> Binary compatibility could be preserve by hacking NGROUPS_MAX to
> NGROUPS_MAX - 1 (ugh).  I don't see how to preserve source level
> compatibility.  You have to change either the semantics by not
> putting the egid in group lists, or NGROUPS_MAX to NGROUPS_MAX+1
> in many places.  Portable applications need the latter change anyway.

BTW, in a second pass, I've found one place where I missed the
obvious change.

Index: kern_prot.c
RCS file: /home/ncvs/src/sys/kern/kern_prot.c,v
retrieving revision 1.93
diff -u -p -r1.93 kern_prot.c
--- kern_prot.c 2001/06/06 13:58:03     1.93
+++ kern_prot.c 2001/06/27 10:11:17
@@ -689,7 +687,7 @@ setgroups(p, uap)
                 * 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;
+               newcred->cr_ngroups = 0;
        } else {
                if ((error = copyin((caddr_t)uap->gidset,
                    (caddr_t)newcred->cr_groups, ngrp * sizeof(gid_t)))) {

Ruslan Ermilov          Oracle Developer/DBA,
[EMAIL PROTECTED]           Sunbay Software AG,
[EMAIL PROTECTED]          FreeBSD committer,
+380.652.512.251        Simferopol, Ukraine

http://www.FreeBSD.org  The Power To Serve
http://www.oracle.com   Enabling The Information Age

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to