pw keeps setting /etc/group to 0600
Baptiste Daroussin
bapt at freebsd.org
Mon Nov 19 22:37:48 UTC 2012
On Mon, Nov 19, 2012 at 11:28:43PM +0100, Mateusz Guzik wrote:
> On Sat, Nov 17, 2012 at 11:20:21AM -0500, Ryan Stone wrote:
> > My original complaint that /etc/group gets permissions of 0600 is a result
> > of a bug in libutil, which bapt@ ported pw to use in r242349. The new
> > group manipulation API using mktemp to create a temporary file, writes the
> > new group database to the temp file and then renames the temp file to
> > /etc/group. The problem here is that mktemp creates a file with a mode of
> > 600, and libutil never chmods it. That should be pretty trivial to fix.
>
> My additional 0,03$:
>
> I took closer look to this and I think that problems are much broader
> than this. I don't know if similar problems were present before.
>
> 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);
>
> 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
>
> Third, I don't like current api.
> gr_lock and gr_tmp have no arguments (that matter anyway)
> gr_copy operates on two descriptors given as arguments
> gr_mkdb takes nothing and is expected to do The Right Thing
gr_mkdb should chmod 0644 after renaming if rename worked.
I should work on this soon.
The API has been design to match the exact same api of pw_utils, I don't like it
either but at least this is consistent.
regards,
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/freebsd-current/attachments/20121119/b4249407/attachment.sig>
More information about the freebsd-current
mailing list