Bug 15421
| Summary: | initgroups(3) spits out messages to stderr; it probably shouldn't | ||
|---|---|---|---|
| Product: | Base System | Reporter: | tobez <tobez> |
| Component: | misc | Assignee: | Anton Berezin <tobez> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | 4.0-CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
Responsible Changed From-To: freebsd-bugs->tobez It's my PR. On Tue, Nov 13, 2001 at 02:28:57PM -0800, Terry Lambert wrote: > Max Khon wrote: > > > > hi, there! > > > > Any objections if I will commit the following patch (see PR/15421)? > > Can setgroups return a positive number? If so, you've just changed > the semantics of the funtion; before, it used to return 0 on 0 or a > positive number. > No. setgroups() is a syscall, and as such returns either 0 or -1. > Also, is removing the _warn() really the only thing you want to > accomplish? It should probably be seperate. > I have intended to commit the below patch for almost a year now, just haven't had enough time to actually fo it. NetBSD runs with this fix since 1999. Index: initgroups.c =================================================================== RCS file: /home/ncvs/src/lib/libc/gen/initgroups.c,v retrieving revision 1.4 diff -u -p -r1.4 initgroups.c --- initgroups.c 2001/08/29 13:52:26 1.4 +++ initgroups.c 2001/11/19 16:16:11 @@ -56,12 +56,6 @@ initgroups(uname, agroup) int groups[NGROUPS], ngroups; ngroups = NGROUPS; - if (getgrouplist(uname, agroup, groups, &ngroups) < 0) - warnx("%s is in too many groups, using first %d", - uname, ngroups); - if (setgroups(ngroups, groups) < 0) { - _warn("setgroups"); - return (-1); - } - return (0); + getgrouplist(uname, agroup, groups, &ngroups); + return (setgroups(ngroups, groups); } Index: initgroups.3 =================================================================== RCS file: /home/ncvs/src/lib/libc/gen/initgroups.3,v retrieving revision 1.10 diff -u -p -r1.10 initgroups.3 --- initgroups.3 2001/10/01 16:08:51 1.10 +++ initgroups.3 2001/11/19 16:16:11 @@ -61,10 +61,14 @@ is automatically included in the groups Typically this value is given as the group number from the password file. .Sh RETURN VALUES +.Rv -std initgroups +.Sh ERRORS The .Fn initgroups -function -returns \-1 if it was not invoked by the super-user. +function may fail and set +.Va errno +for any of the errors specified for the library function +.Xr setgroups 2 . .Sh SEE ALSO .Xr setgroups 2 , .Xr getgrouplist 3 Cheers, -- 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 State Changed From-To: open->closed A different fix committed. |
On two occasions, initgroups(3) uses warn()/warnx() to print messages on the standard error. The first one might be justified (`user is in too many groups'), the second one, which reports the failure of the call to setgroups(2), is definitely not: in this case initgroups(3) returns -1, and errno will be set by setgroups(2), so the caller is able to perform a check on its own. This warning bites, for example, when CVS in pserver mode is run as non-root: the warning gets printed due to EPERM, and the CVS client gets confused by unexpected output. If, for whatever reasons, you decide to keep this warning, please do fix the manpage, which does not say a thing about such behavior. A good example is the getbsize(3) manpage, it clearly states the possibility of writing warning messages. Fix: The fix I prefer (note that the first warning stays where it is): Cheers, Anton.--upcb82AZcLTcs3tIlYhhqbWM7Uo7tM8OxDFJEOsaCwQrSBfx Content-Type: text/plain; name="file.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="file.diff" --- /usr/src/lib/libc/gen/initgroups.c Fri Jul 12 20:53:56 1996 +++ initgroups.c Sat Dec 11 20:30:54 1999 @@ -53,7 +53,6 @@ warnx("%s is in too many groups, using first %d", uname, ngroups); if (setgroups(ngroups, groups) < 0) { - warn("setgroups"); return (-1); } return (0); --- /usr/src/lib/libc/gen/initgroups.3 Sun Aug 29 12:40:15 1999 +++ initgroups.3 Sat Dec 11 20:42:03 1999 @@ -58,6 +58,13 @@ is automatically included in the groups list. Typically this value is given as the group number from the password file. +.Pp +If the user is in too many groups, a warning message is written +to standard error and the first +.Dv NGROUPS , +as defined in +.Ao Pa sys/param.h Ac , +are used. .Sh RETURN VALUES The .Fn initgroups How-To-Repeat: Compile and run as non-root this program: #include <errno.h> #include <unistd.h> #include <sys/types.h> #include <pwd.h> int main( void) { struct passwd *pw = getpwuid( getuid()); if (!pw) errc( 1, errno, "getpwuid() failed"); if ( initgroups( pw-> pw_name, pw-> pw_gid) < 0) errc( 1, errno, "initgroups(3) failed"); return 0; } You should get this: initg: setgroups: Operation not permitted initg: initgroups(3) failed: Operation not permitted