svn commit: r244735 - head/lib/libutil

[Baptiste Daroussin]

On Thu, Dec 27, 2012 at 09:21:13PM +0100, Jilles Tjoelker wrote:
> 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.

done concerning the |O_CLOEXEC

Thanks,
Bapt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20121227/cc7affd1/attachment.sig>