From 953e54b3c1853cbc2a959f96455184ffb137b124 Mon Sep 17 00:00:00 2001 From: Nate Williams Date: Thu, 16 Jul 1998 17:18:25 +0000 Subject: 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 --- pw/edgroup.c | 4 ++-- pw/fileupd.c | 17 ++++++++++++++--- pw/pw.c | 25 +++++++++++++++++++++++-- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/pw/edgroup.c b/pw/edgroup.c index c927b06..6116fa6 100644 --- 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 @@ -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 3782bf7..fe46480 100644 --- 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 @@ -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 634716e..5daa213 100644 --- 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 +#include #include #include @@ -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) { -- cgit v1.2.3-56-ge451