From owner-freebsd-current@FreeBSD.ORG  Wed Nov 21 15:45:55 2012
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Delivered-To: freebsd-current@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52])
 by hub.freebsd.org (Postfix) with ESMTP id A9E9E91E;
 Wed, 21 Nov 2012 15:45:55 +0000 (UTC) (envelope-from jh@FreeBSD.org)
Received: from gw02.mail.saunalahti.fi (gw02.mail.saunalahti.fi
 [195.197.172.116])
 by mx1.freebsd.org (Postfix) with ESMTP id 1714E8FC19;
 Wed, 21 Nov 2012 15:45:55 +0000 (UTC)
Received: from a91-153-116-96.elisa-laajakaista.fi
 (a91-153-116-96.elisa-laajakaista.fi [91.153.116.96])
 by gw02.mail.saunalahti.fi (Postfix) with SMTP id 36381139511;
 Wed, 21 Nov 2012 17:45:43 +0200 (EET)
Date: Wed, 21 Nov 2012 17:45:43 +0200
From: Jaakko Heinonen <jh@FreeBSD.org>
To: Mateusz Guzik <mjguzik@gmail.com>, Ryan Stone <rysto32@gmail.com>,
 FreeBSD Current <freebsd-current@freebsd.org>, bapt@freebsd.org
Subject: Re: pw keeps setting /etc/group to 0600
Message-ID: <20121121154542.GA1849@a91-153-116-96.elisa-laajakaista.fi>
References: <CAFMmRNxDr=+psiazVrJ8e=T4fogiiPv5nEAo+nfoD=tPYMehCw@mail.gmail.com>
 <CAFMmRNxW0FzupbC9w4U5pPZUoqOE+3rqFNRUrnsMRGFnO7qPWA@mail.gmail.com>
 <20121119222843.GB22292@dft-labs.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20121119222843.GB22292@dft-labs.eu>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-BeenThere: freebsd-current@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: Discussions about the use of FreeBSD-current
 <freebsd-current.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-current>, 
 <mailto:freebsd-current-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-current>
List-Post: <mailto:freebsd-current@freebsd.org>
List-Help: <mailto:freebsd-current-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-current>,
 <mailto:freebsd-current-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 21 Nov 2012 15:45:55 -0000

On 2012-11-19, Mateusz Guzik wrote:
> First, pw should not fail if other instance is running, it should wait
> instead (think of parallel batch scripts adding some users/groups).
> 
> Second, current code has a race:
> 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) {
> [..]
> gr_copy(pfd, tfd, gr, old_gr); /* copy from groupfile to tempfile */
> [..]
> rename(tempfile,groupfile);

Hmm, could using the O_EXLOCK flag for open() instead of flock() help here?

> Now let's consider threads A and B:
> 
> A: open()
> A: lock();
> A: gr_copy
> B: open()
> 
> Now B has file descriptor to /etc/group that is about to be removed.
> 
> A: rename()
> A: unlock()
> B: lock()
> 
> Now B has a lock on unlinked file.
> 
> B: gr_copy()
> B: rename()
> 
> ... and stores new content losing modifications done by A

-- 
Jaakko