Date: Thu, 27 Dec 2012 21:21:13 +0100 From: Jilles Tjoelker <jilles@stack.nl> To: Baptiste Daroussin <bapt@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r244735 - head/lib/libutil Message-ID: <20121227202113.GA36622@stack.nl> In-Reply-To: <201212271409.qBRE9ogT092286@svn.freebsd.org> References: <201212271409.qBRE9ogT092286@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Dec 27, 2012 at 02:09:50PM +0000, Baptiste Daroussin wrote: > Author: bapt > Date: Thu Dec 27 14:09:50 2012 > New Revision: 244735 > URL: http://svnweb.freebsd.org/changeset/base/244735 > Log: > Use flopen(3) instead of open(2) + flock(2) > Modified: > head/lib/libutil/gr_util.c > head/lib/libutil/pw_util.c > Modified: head/lib/libutil/gr_util.c > ============================================================================== > --- head/lib/libutil/gr_util.c Thu Dec 27 13:21:37 2012 (r244734) > +++ head/lib/libutil/gr_util.c Thu Dec 27 14:09:50 2012 (r244735) > @@ -106,10 +106,8 @@ gr_lock(void) > for (;;) { > struct stat st; > > - lockfd = open(group_file, O_RDONLY, 0); > - if (lockfd < 0 || fcntl(lockfd, F_SETFD, 1) == -1) > - err(1, "%s", group_file); > - if (flock(lockfd, LOCK_EX|LOCK_NB) == -1) { > + lockfd = flopen(group_file, O_RDONLY|O_NONBLOCK, 0); > + if (lockfd == -1) { > if (errno == EWOULDBLOCK) { > errx(1, "the group file is busy"); > } else { > > Modified: head/lib/libutil/pw_util.c > ============================================================================== > --- head/lib/libutil/pw_util.c Thu Dec 27 13:21:37 2012 (r244734) > +++ head/lib/libutil/pw_util.c Thu Dec 27 14:09:50 2012 (r244735) > @@ -179,11 +179,8 @@ pw_lock(void) > for (;;) { > struct stat st; > > - lockfd = open(masterpasswd, O_RDONLY, 0); > - if (lockfd < 0 || fcntl(lockfd, F_SETFD, 1) == -1) > - err(1, "%s", masterpasswd); > - /* XXX vulnerable to race conditions */ > - if (flock(lockfd, LOCK_EX|LOCK_NB) == -1) { > + lockfd = flopen(masterpasswd, O_RDONLY|O_NONBLOCK, 0); > + if (lockfd == -1) { > if (errno == EWOULDBLOCK) { > errx(1, "the password db file is busy"); > } else { The fcntl(lockfd, F_SETFD, 1) was removed and not replaced by anything. The comment in pw_util.c gives the impression that this is vital for security; in fact, chpass does not keep the password file locked across a user's editor. However, the idea of setting close-on-exec on this file descriptor is good. Please add |O_CLOEXEC to the flopen() flags in both cases. -- Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121227202113.GA36622>