Date: Wed, 21 Nov 2012 17:27:18 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Jaakko Heinonen <jh@FreeBSD.org> Cc: bapt@freebsd.org, FreeBSD Current <freebsd-current@freebsd.org>, Ryan Stone <rysto32@gmail.com> Subject: Re: pw keeps setting /etc/group to 0600 Message-ID: <20121121162718.GA10768@dft-labs.eu> In-Reply-To: <20121121154542.GA1849@a91-153-116-96.elisa-laajakaista.fi> References: <CAFMmRNxDr=%2BpsiazVrJ8e=T4fogiiPv5nEAo%2BnfoD=tPYMehCw@mail.gmail.com> <CAFMmRNxW0FzupbC9w4U5pPZUoqOE%2B3rqFNRUrnsMRGFnO7qPWA@mail.gmail.com> <20121119222843.GB22292@dft-labs.eu> <20121121154542.GA1849@a91-153-116-96.elisa-laajakaista.fi>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Nov 21, 2012 at 05:45:43PM +0200, Jaakko Heinonen wrote:
> 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?
>
Yes, this would fix the race.
But the problem of pw exiting due to other process holding the lock
remains. And I think that fixing it will require holding a lock over
whole time pw is running so that we have stable snapshot of user base at
least in regard of local files.
One could create one lock, say /etc/.pw.lock, that would be used to
synchronize any changes to /etc/master.passwd, /etc/group and whatnot.
And then there is this API issue (but maybe this is just me
nitpicking).
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121121162718.GA10768>