Date: Fri, 17 May 2002 18:40:57 -0400 (EDT) From: "Geoffrey C. Speicher" <geoff@sea-incorporated.com> To: freebsd-stable@freebsd.org Cc: "Matthew D. Fuller" <fullermd@over-yonder.net>, Matt Simerson <freebsd@blockads.com> Subject: bug in pw, -STABLE [patch] Message-ID: <Pine.BSF.4.10.10205171825320.67053-300000@sea-incorporated.com>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
All:
I posted a message the other day to -hackers with a patch for pw(8). The
patch corrects a bug where running multiple instances of pw quickly leads
to corruption of master.passwd.
I just now realized that I copy-n-pasted those patches into my email, so
they are not very useful. Besides that, I wasn't really sure where the
post should go anyway, so I'm reposting here with good patches attached
and the original message (slightly modified) below.
I have a patched binary on a production server with several thousand
accounts, and it is working flawlessly. I need a committer to take a peek
at these patches and either commit them or tell me what needs to be
changed before they can be committed. Some points for consideration are:
1. The lock isn't very fine-grained. We grab one giant lock before
doing any operations at all, and let it go right before we exit.
The alternative is to lock for /etc/master.passwd and /etc/group
individually as necessary, but I saw no point in going that far since
pw seems to spend most of its time updating those files anyway.
2. The lockfile is /var/run/pw.lock and it always contains the pid of
the pw process that most recently grabbed the lock.
3. The lockfile is never deleted, because doing so seemed to cause a
race condition between the time the file was closed and unlinked,
leading to password file corruption again. If anyone has a solution
to this, please speak up.
4. To test, backup your /etc/master.passwd and /etc/group, and then try
something like this, several times with the old pw binary to verify
consistent breakage, and then several times with the patched binary to
verify that it doesn't break anymore.
In one terminal, run this first:
#!/bin/sh
i=0
while [ $i -le 1000 ]; do
pw add user testuser$i
i=$(($i+1))
done
In another terminal, wait about 10-15 seconds and then run this
(or tail -f /etc/master.passwd and run it when testuser500 shows up):
#!/bin/sh
i=0
while [ $i -le 500 ]; do
pw del user testuser$i
i=$(($i+1))
done
Geoff
[-- Attachment #2 --]
--- src/usr.sbin/pw/pw.c.orig Wed May 15 13:35:21 2002
+++ src/usr.sbin/pw/pw.c Wed May 15 13:37:53 2002
@@ -98,10 +98,12 @@
main(int argc, char *argv[])
{
int ch;
+ int lockfd;
int mode = -1;
int which = -1;
char *config = NULL;
struct userconf *cnf;
+ char *pidstr;
static const char *opts[W_NUM][M_NUM] =
{
@@ -202,6 +204,17 @@
errx(EX_NOPERM, "you must be root to run this program");
/*
+ * Gain exclusive lock before updating any files
+ */
+ lockfd = open(_PWLOCK, O_WRONLY | O_CREAT | O_EXLOCK, _LOCK_FILE_MODE);
+ if (lockfd == -1)
+ err(EX_UNAVAILABLE, "%s", _PWLOCK);
+
+ ftruncate(lockfd,0);
+ write(lockfd, pidstr, asprintf(&pidstr, "%d", getpid()));
+ free(pidstr);
+
+ /*
* We should immediately look for the -q 'quiet' switch so that we
* don't bother with extraneous errors
*/
@@ -226,7 +239,7 @@
setgrdir(etcpath);
}
}
-
+
/*
* Now, let's do the common initialisation
*/
@@ -259,6 +272,12 @@
pw_log(cnf, mode, which, "NIS maps updated");
}
}
+
+ /*
+ * Release the lock
+ */
+ close(lockfd);
+
return ch;
}
[-- Attachment #3 --]
--- src/usr.sbin/pw/pwupd.h.orig Tue May 14 22:06:37 2002
+++ src/usr.sbin/pw/pwupd.h Wed May 15 13:35:14 2002
@@ -110,6 +110,12 @@
#ifndef _GROUP
#define _GROUP "group"
#endif
+#ifndef _PWLOCK
+#define _PWLOCK "/var/run/pw.lock"
+#endif
+#ifndef _LOCK_FILE_MODE
+#define _LOCK_FILE_MODE (S_IRUSR | S_IWUSR)
+#endif
__BEGIN_DECLS
int addpwent __P((struct passwd * pwd));
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.10.10205171825320.67053-300000>