From owner-svn-src-all@FreeBSD.ORG Thu Dec 27 20:21:17 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id DF0084AB; Thu, 27 Dec 2012 20:21:17 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5012::107]) by mx1.freebsd.org (Postfix) with ESMTP id 4795A8FC08; Thu, 27 Dec 2012 20:21:17 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id AE4D7120209; Thu, 27 Dec 2012 21:21:13 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 89C332848C; Thu, 27 Dec 2012 21:21:13 +0100 (CET) Date: Thu, 27 Dec 2012 21:21:13 +0100 From: Jilles Tjoelker To: Baptiste Daroussin Subject: Re: svn commit: r244735 - head/lib/libutil Message-ID: <20121227202113.GA36622@stack.nl> References: <201212271409.qBRE9ogT092286@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201212271409.qBRE9ogT092286@svn.freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Dec 2012 20:21:18 -0000 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