From owner-freebsd-current Wed Jun 27 3:41:31 2001 Delivered-To: freebsd-current@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id 5159037B409; Wed, 27 Jun 2001 03:41:17 -0700 (PDT) (envelope-from ru@whale.sunbay.crimea.ua) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.2/8.11.2) id f5RAeid49078; Wed, 27 Jun 2001 13:40:44 +0300 (EEST) (envelope-from ru) Date: Wed, 27 Jun 2001 13:40:44 +0300 From: Ruslan Ermilov To: Bruce Evans Cc: Robert Watson , arch@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: [CFR] ucred.cr_gid Message-ID: <20010627134044.A23159@sunbay.com> Mail-Followup-To: Bruce Evans , Robert Watson , arch@FreeBSD.ORG, current@FreeBSD.ORG References: <20010627100905.A2097@sunbay.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: ; from bde@zeta.org.au on Wed, Jun 27, 2001 at 06:40:53PM +1000 Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org 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 majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message