Bug 23501

Summary:	pw destroy /etc/master.passwd when pw executing at the same time
Product:	Base System	Reporter:	Tomonobu AKIMOTO <akimoto>
Component:	bin	Assignee:	kensmith
Status:	Closed FIXED
Severity:	Affects Only Me
Priority:	Normal
Version:	4.2-RELEASE
Hardware:	Any
OS:	Any

Description Tomonobu AKIMOTO 2000-12-12 14:20:00 UTC

/etc/master.passwd is often broken on the servers.
Sometimes,we lost many users from the begining include of root,
sometimes,many users from the bottom.
So,we test.
We make 10000 users on testing server,then we execute 3 pw commands
at the same time.
few seconds later,/etc/master.passwd is broken.

Comment 1 dhagan 2001-01-04 21:13:50 UTC

This is almost certainly the fault of fileupdate() in fileupd.c:72.  The
logic is too byzantine for me to figure out right now, but the comment
at line 181 seems worrisome.

Daniel

Comment 2 Tomonobu AKIMOTO 2001-01-05 05:36:09 UTC

Hello,Daniel

Thank you for your replying.
But I can't figure out,too.

If anyone knows how to modify concretely,
please tell me that.

On Thu, 04 Jan 2001 16:13:50 -0500
Daniel Hagan <dhagan@colltech.com> wrote:

> This is almost certainly the fault of fileupdate() in fileupd.c:72.  The
> logic is too byzantine for me to figure out right now, but the comment
> at line 181 seems worrisome.
> 
> Daniel

Comment 3 Alex Kapranoff 2001-01-12 17:47:34 UTC

>  If anyone knows how to modify concretely,
>  please tell me that.
>  
>  On Thu, 04 Jan 2001 16:13:50 -0500
>  Daniel Hagan <dhagan@colltech.com> wrote:
>  
>  > This is almost certainly the fault of fileupdate() in fileupd.c:72.  The
>  > logic is too byzantine for me to figure out right now, but the comment
>  > at line 181 seems worrisome.
>  > 
>  > Daniel

Tomonobo,
try the following patch and see if it helps.

diff -ru /usr/src/usr.sbin/pw/edgroup.c ./edgroup.c
--- /usr/src/usr.sbin/pw/edgroup.c	Thu Dec 14 22:13:45 2000
+++ ./edgroup.c	Fri Jan 12 20:21:42 2001
@@ -68,7 +68,7 @@
 	strcpy(grouptmp, groupfile);
 	strcat(grouptmp, ".new");
 
-	if ((infd = open(groupfile, O_RDWR | O_CREAT, 0644)) != -1) {
+	if ((infd = open(groupfile, O_RDWR | O_CREAT | O_EXLOCK, 0644)) != -1) {
 		FILE           *infp;
 
 		if ((infp = fdopen(infd, "r+")) == NULL)
@@ -76,7 +76,7 @@
 		else {
 			int             outfd;
 
-			if ((outfd = open(grouptmp, O_RDWR | O_CREAT | O_TRUNC | O_EXLOCK, 0644)) != -1) {
+			if ((outfd = open(grouptmp, O_RDWR | O_CREAT | O_TRUNC, 0644)) != -1) {
 				FILE           *outfp;
 
 				if ((outfp = fdopen(outfd, "w+")) == NULL)
@@ -207,8 +207,7 @@
 
 							/*
 							 * This is a gross hack, but we may have corrupted the
-							 * original file. Unfortunately, it will lose preservation
-							 * of the inode.
+							 * original file.
 							 */
 							if (fflush(infp) == EOF || ferror(infp))
 								rc = rename(grouptmp, groupfile) == 0;
diff -ru /usr/src/usr.sbin/pw/fileupd.c ./fileupd.c
--- /usr/src/usr.sbin/pw/fileupd.c	Thu Dec 14 22:13:45 2000
+++ ./fileupd.c	Fri Jan 12 20:20:44 2001
@@ -76,7 +76,7 @@
 	if (pfxlen <= 1)
 		rc = EINVAL;
 	else {
-		int    infd = open(filename, O_RDWR | O_CREAT, fmode);
+		int    infd = open(filename, O_RDWR | O_CREAT | O_EXLOCK, fmode);
 
 		if (infd == -1)
 			rc = errno;
@@ -92,7 +92,7 @@
 
 				strcpy(file, filename);
 				strcat(file, ".new");
-				outfd = open(file, O_RDWR | O_CREAT | O_TRUNC | O_EXLOCK, fmode);
+				outfd = open(file, O_RDWR | O_CREAT | O_TRUNC, fmode);
 				if (outfd == -1)
 					rc = errno;
 				else {
@@ -183,8 +183,6 @@
 								 * to 'file'.
 								 * This is a gross hack, but we may have
 								 * corrupted the original file
-								 * Unfortunately, it will lose the inode
-								 * and hence the lock.
 								 */
 								if (fflush(infp) == EOF || ferror(infp))
 									rename(file, filename);

-- 
Alex Kapranoff,                              Voice: +7(0832)791845
We've lived 11 days in the brand new millenium...

Comment 4 Tomonobu AKIMOTO 2001-01-17 07:11:31 UTC

Hello,Alex.

I tried your patch and tested it.
Then the problem is solved.

Thanks a lot.

> try the following patch and see if it helps.

Comment 5 jontow 2001-06-22 15:48:07 UTC

This PR can be closed

-- 

- Jonathan Towne/jontow@twcny.rr.com

Comment 6 Poul-Henning Kamp freebsd_committer

2001-06-22 16:43:09 UTC

State Changed
From-To: open->closed

Done.

Comment 7 steve 2003-12-30 20:39:13 UTC

I was wondering if someone could please confirm that this patch was
added to FreeBSD 4.9-RELEASE, as I am still seeing the same issues.

Thanks,
Steve

Comment 8 Andy Farkas 2003-12-30 21:36:03 UTC

This PR was closed for no reason.

The patch was not applied.

Comment 9 Simon L. B. Nielsen freebsd_committer

2003-12-30 22:20:46 UTC

State Changed
From-To: closed->open

Reopen PR, since users report that the problem still exists, and from 
looking in the cvs logs the patch in the PR has not been applied.

Comment 10 kensmith freebsd_committer

2004-02-23 02:36:33 UTC

Responsible Changed
From-To: freebsd-bugs->kensmith


I'll give this one a try.  The submitted patch looks like it solves 
what might be a file library routine buffering issue/file locking 
issue but pw(8) might need more work than just that.  I looked it over 
a bit and can't see where it is using normal password file locking 
to avoid problems with vipw(8) running at the same time, etc.

Comment 11 kensmith freebsd_committer

2004-03-08 20:44:37 UTC

State Changed
From-To: open->closed


Patch was committed.  As part of checking over the patch I verified that 
shifting the locking to the source file instead of the temp file does 
also seem to bring pw(8) into agreement with other master.passwd related 
things like PAM, vipw(8), etc. - they all seem to lock master.passwd itself.