From owner-freebsd-hackers Sun Jun 23 14: 6:56 2002 Delivered-To: freebsd-hackers@freebsd.org Received: from hardtime.linuxman.net (hardtime.linuxman.net [66.147.26.65]) by hub.freebsd.org (Postfix) with ESMTP id 9DD9737B410 for ; Sun, 23 Jun 2002 14:06:40 -0700 (PDT) Received: from mortis.over-yonder.net (localhost [127.0.0.1]) by hardtime.linuxman.net (8.11.6/8.11.6) with ESMTP id g5NKCPB02545; Sun, 23 Jun 2002 15:12:26 -0500 Received: by mortis.over-yonder.net (Postfix, from userid 100) id F0FA91F05; Sun, 23 Jun 2002 16:06:36 -0500 (CDT) Date: Sun, 23 Jun 2002 16:06:36 -0500 From: "Matthew D. Fuller" To: Paul Herman Cc: "Geoffrey C. Speicher" , freebsd-hackers@FreeBSD.ORG Subject: Re: bug in pw, -STABLE [patch] Message-ID: <20020623210636.GK81018@over-yonder.net> References: <20020623192457.GJ81018@over-yonder.net> <20020623124243.H38673-100000@mammoth.eat.frenchfries.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020623124243.H38673-100000@mammoth.eat.frenchfries.net> User-Agent: Mutt/1.4i-fullermd.1 X-Editor: vi X-OS: FreeBSD Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sun, Jun 23, 2002 at 01:19:02PM -0700 I heard the voice of Paul Herman, and lo! it spake thus: > > I think you might have your infp confused with your outfp. It's > not writing to the original "live" file, it's just writing the new > temp file. That part of the code is OK. I'm talking about down around line 177 thru (unpatched), where it's copying back to the primary file. As it stands now (I hadn't realized in my prior reading) it's line-by-line'ing it: /* * Copy data back into the * original file and truncate */ rewind(infp); rewind(outfp); while (fgets(line, linesize, outfp) != NULL) 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 * and hence the lock. */ if (fflush(infp) == EOF || ferror(infp)) rename(file, filename); else ftruncate(infd, ftell(infp)); which is Very Bad (tm) in that it's not very resilient against system crashes. The rename() solution is much safer. > As for the giant lock, it would be like closing down the entire > airport after someone finds a knife in the snack bar, and in my > opinion an this is an unwise and brutal thing to do. No, it's more like closing the snack bar while you clean it, rather than closing the first steam tray while cleaning that, then the second steam tray, then the third booth on the left side, then... That way, when you open the snack bar, you know that the whole thing is clean (or, depending on your skill and work ethic, at least equally dirty ;). In many passwd-manipulations, the file can't be safely modified in-place, so it has to be done out-of-place, then atomically shoved in, which requires a rename() or the ilk; the downside is that you can't flock() the file in question and handle that easily. One also has to consider the impact with other programs, potentially not written by us, that modify things; a wrapper lock on "changes to the auth information" is really the only way to ensure consistency; otherwise, you can't REALLY be sure that the passwd and group files are in sync, to say nothing of preventing crashes from mangling things. > Not to mention stale /var/run/ files that might be lying around... See the code I put in pid_begin() to handle just that case, that being the reason the function was written in the first place (otherwise, I could dispose of a lot of mess, and just use an empty file as a mutex for it). > I do appreciate your work, but please don't commit this until the > real issue is solved, and the dust settles. There still may be > some use for it... plenty of time until the next release cycle. Commit? How would I do that? :P -- Matthew Fuller (MF4839) | fullermd@over-yonder.net Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ "The only reason I'm burning my candle at both ends, is because I haven't figured out how to light the middle yet" To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message