Fix race condition in pw caused by multiple instances of pwd_mkdb being

run at the same time.

Notes:
    The fileupdate function is still somewhat broken.  Instead of
    returning a failure code if it can't modify the original file it
    renames the .new file and continues as though nothing is wrong.
    This will cause the lock on the original file to be lost and could
    lead to a similar race condition.  I left that portion of the code
    alone since I feel that the maintainer of the code would have a
    better concept of how he wants to handle errors in that function
    than I do.

PR:		bin/6787
Submitted by:	Craig Spannring <cts@internetcds.com>

diff --git a/pw/edgroup.c b/pw/edgroup.c
index c927b06cc30a383b34d7fa22fca4aa55e3a89d21..6116fa6854ac4e4269225d0f215dabbc6d75a526 100644 (file)
--- a/pw/edgroup.c
+++ b/pw/edgroup.c
@@ -26,7 +26,7 @@
 
 #ifndef lint
 static const char rcsid[] =
-       "$Id$";
+       "$Id: edgroup.c,v 1.5 1997/10/10 06:23:30 charnier Exp $";
 #endif /* not lint */
 
 #include <stdio.h>
@@ -64,7 +64,7 @@ editgroups(char *name, char **groups)
        int             rc = 0;
        int             infd;
 
-       if ((infd = open(groupfile, O_RDWR | O_CREAT | O_EXLOCK, 0644)) != -1) {
+       if ((infd = open(groupfile, O_RDWR | O_CREAT, 0644)) != -1) {
                FILE           *infp;
 
                if ((infp = fdopen(infd, "r+")) == NULL)

diff --git a/pw/fileupd.c b/pw/fileupd.c
index 3782bf7d1da4ed13a9acb2754dee02f97b8da62d..fe46480048d1280bd5b1708eee5253d2a9e25072 100644 (file)
--- a/pw/fileupd.c
+++ b/pw/fileupd.c
@@ -26,7 +26,7 @@
 
 #ifndef lint
 static const char rcsid[] =
-       "$Id$";
+       "$Id: fileupd.c,v 1.5 1997/10/10 06:23:31 charnier Exp $";
 #endif /* not lint */
 
 #include <stdio.h>
@@ -76,7 +76,7 @@ fileupdate(char const * filename, mode_t fmode, char const * newline, char const
        if (pfxlen <= 1)
                errno = EINVAL;
        else {
-               int             infd = open(filename, O_RDWR | O_CREAT | O_EXLOCK, fmode);
+               int             infd = open(filename, O_RDWR | O_CREAT, fmode);
 
                if (infd != -1) {
                        FILE           *infp = fdopen(infd, "r+");
@@ -168,9 +168,20 @@ fileupdate(char const * filename, mode_t fmode, char const * newline, char const
                                                                        fputs(line, infp);
 
                                                                /*
+                                                                 * If there was a problem with copying
+                                                                 * we will just rename 'file.new' 
+                                                                 * to 'file'.
                                                                 * This is a gross hack, but we may have
                                                                 * corrupted the original file
-                                                                * Unfortunately, it will lose the inode.
+                                                                * Unfortunately, it will lose the inode
+                                                                 * and hence the lock.
+                                                                 *
+                                                                 * The implications of this is that this invocation of pw 
+                                                                 * won't have the file locked and concurrent copies
+                                                                 * of pw, vipw etc could clobber what this one is doing.
+                                                                 *
+                                                                 * It should probably just return an error instead
+                                                                 * of going on like nothing is wrong.
                                                                 */
                                                                if (fflush(infp) == EOF || ferror(infp))
                                                                        rc = rename(file, filename) == 0;

diff --git a/pw/pw.c b/pw/pw.c
index 634716e5208dc9a06511ed9f5d21a3679125669f..5daa213c880b0133f116a167f7c3e83cd1008ad7 100644 (file)
--- a/pw/pw.c
+++ b/pw/pw.c
@@ -26,11 +26,12 @@
 
 #ifndef lint
 static const char rcsid[] =
-       "$Id$";
+       "$Id: pw.c,v 1.7 1997/10/10 06:23:34 charnier Exp $";
 #endif /* not lint */
 
 #include "pw.h"
 #include <err.h>
+#include <fcntl.h>
 #include <paths.h>
 #include <sys/wait.h>
 
@@ -49,6 +50,7 @@ static struct cargs arglist;
 
 static int      getindex(const char *words[], const char *word);
 static void     cmdhelp(int mode, int which);
+static int      filelock(const char *filename);
 
 
 int
@@ -147,7 +149,20 @@ main(int argc, char *argv[])
         * Now, let's do the common initialisation
         */
        cnf = read_userconfig(getarg(&arglist, 'C') ? getarg(&arglist, 'C')->val : NULL);
-       ch = funcs[which] (cnf, mode, &arglist);
+
+
+       /*
+        * Be pessimistic and lock the master passowrd and group
+        * files right away.  Keep it locked for the duration.
+        */
+       if (-1 == filelock(_PATH_GROUP) || -1 == filelock(_PATH_MASTERPASSWD))
+       {
+               ch = EX_IOERR;
+       }
+       else
+       {
+               ch = funcs[which] (cnf, mode, &arglist);
+       }       
 
        /*
         * If everything went ok, and we've been asked to update
@@ -177,6 +192,12 @@ main(int argc, char *argv[])
        return ch;
 }
 
+static int
+filelock(const char *filename)
+{
+       return open(filename, O_RDONLY | O_EXLOCK, 0);
+}
+
 static int
 getindex(const char *words[], const char *word)
 {