Date: Tue, 20 Nov 2001 15:43:52 +0100 From: Anton Berezin <tobez@tobez.org> To: Ruslan Ermilov <ru@FreeBSD.org> Cc: Max Khon <fjoe@iclub.nsu.ru>, current@FreeBSD.org Subject: Re: misc/15421 (was: Re: initgroups) Message-ID: <20011120154352.D36033@heechee.tobez.org> In-Reply-To: <20011120162703.A34879@sunbay.com>; from ru@FreeBSD.org on Tue, Nov 20, 2001 at 04:27:03PM %2B0200 References: <20011114021956.B10325@iclub.nsu.ru> <3BF19EA9.3FC5F040@mindspring.com> <20011119181949.R32927@sunbay.com> <20011119222854.B38492@iclub.nsu.ru> <20011120150239.D7645@sunbay.com> <20011120151250.C36033@heechee.tobez.org> <20011120162703.A34879@sunbay.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Nov 20, 2001 at 04:27:03PM +0200, Ruslan Ermilov wrote: > On Tue, Nov 20, 2001 at 03:12:50PM +0100, Anton Berezin wrote: > > On Tue, Nov 20, 2001 at 03:02:39PM +0200, Ruslan Ermilov wrote: > > > While this is indeed a good thing to do, this is completely > > > unrelated to the above mentioned problem, and should be done > > > separately. Here's the list of src/ files that do not check the > > > return value of initgroups(3), and may need to be fixed, but some > > > of them explicitly ignore the result to indicate the fact they > > > consider this error non-fatal. > > > libexec/ftpd/ftpd.c > > > libexec/rexecd/rexecd.c > > > usr.bin/calendar/calendar.c > > > usr.sbin/inetd/inetd.c > > > > There used to be *many* more problematic files. Please see > > > > http://www.freebsd.org/cgi/getmsg.cgi?fetch=801566+0+/usr/local/www/db/text/2001/freebsd-stable/20010722.freebsd-stable > > > > To my knowledge, only printjob.c was fixed, though I have not looked > > into every file in the list since then. > Yes, but I specifically left contrib/ and crypto/ files, and files that > do not check the result of other calls like setgrp() etc. We do not want to omit contrib/ files, since the whole hoopla started because of the contrib/cvs/. > > But as I said in the private message, I do not feel strongly about > > this, and I think that the fix can be safely committed. I do not > > think these things are quite unrelated, though. :-) > Not checking the return value is always BAD except when (not) done > intentionally (flagged by a(void)ing the return value of a function), > whether or not a function in question prints some diagnostic output on > standard error; that's why I still think these problems are in fact > unrelated. :-) In this case your own version of the fix should be modified from + getgrouplist(uname, agroup, groups, &ngroups); + return (setgroups(ngroups, groups); to + (void) getgrouplist(uname, agroup, groups, &ngroups); + return (setgroups(ngroups, groups); , to be pedantic. :-) The point I am trying to (not very strongly) make is that we at least have some indication that there is a problem with the current behavior (with the exception of the daemons with closed/redirected to /dev/null stderr). By (rightfully) fixing initgroups(), we loose even this precious little diagnostic we have. That's why initgroups() fix and the code audit are probably best done at the same time - unless we can guarantee the audit part will not be forgotten. Cheers, $Anton. -- | Anton Berezin | FreeBSD: The power to serve | | catpipe Systems ApS _ _ |_ | http://www.FreeBSD.org | | tobez@catpipe.net (_(_|| | tobez@FreeBSD.org | | +45 7021 0050 | Private: tobez@tobez.org | To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011120154352.D36033>