From owner-freebsd-bugs Mon Jun 24 23: 0:28 2002 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 98C3E37B405 for ; Mon, 24 Jun 2002 23:00:06 -0700 (PDT) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.6/8.11.6) id g5P606q08404; Mon, 24 Jun 2002 23:00:06 -0700 (PDT) (envelope-from gnats) Date: Mon, 24 Jun 2002 23:00:06 -0700 (PDT) Message-Id: <200206250600.g5P606q08404@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Paul Herman Subject: Re: bin/38676 change request for pw command Reply-To: Paul Herman Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org The following reply was made to PR bin/38676; it has been noted by GNATS. From: Paul Herman To: FreeBSD-gnats-submit@FreeBSD.ORG Cc: Takumi ISHII , "Geoffrey C. Speicher" , "Matthew D. Fuller" Subject: Re: bin/38676 change request for pw command Date: Mon, 24 Jun 2002 22:52:50 -0700 (PDT) Here is an better version of the patch in bin/23501. In addition to the advisory lock, link counts are checked to avoid any unlink/rename issues. This has been tested by myself and seems to also fix the problem. The topic of HOW to best fix this bug has recently been hashed out a bit on -hackers and -stable. Matthew Fuller proposed an alternate fix by using a giant lock in /var/run/, so the jury is still out on the best way to solve the problem. I'll let Matthew speak for himself, but I feel that the either the patch in bin/23501 or the following improved patch is the best way to solve the problem. Opinions in the audit-trail are surely to follow. :-) -Paul. Index: edgroup.c =================================================================== RCS file: /u02/ncvs/src/usr.sbin/pw/edgroup.c,v retrieving revision 1.8 diff -u -r1.8 edgroup.c --- edgroup.c 28 Aug 1999 01:19:16 -0000 1.8 +++ edgroup.c 25 Jun 2002 05:27:49 -0000 @@ -68,7 +68,26 @@ strcpy(grouptmp, groupfile); strcat(grouptmp, ".new"); - if ((infd = open(groupfile, O_RDWR | O_CREAT, 0644)) != -1) { + /* + * Open and lock the original group file. We check the link count + * in order to avoid any link/unlink/remove race. + */ + for (;;) { + struct stat st; + + infd = open(groupfile, O_RDWR | O_CREAT | O_EXLOCK, 0644); + if (infd == -1) + break; + if (fstat(infd, &st) < 0) { + close(infd); + return -1; + } + if (st.st_nlink == 1) + break; + close(infd); + } + + if (infd != -1) { FILE *infp; if ((infp = fdopen(infd, "r+")) == NULL) Index: fileupd.c =================================================================== RCS file: /u02/ncvs/src/usr.sbin/pw/fileupd.c,v retrieving revision 1.9 diff -u -r1.9 fileupd.c --- fileupd.c 26 Oct 1999 04:27:13 -0000 1.9 +++ fileupd.c 25 Jun 2002 04:15:08 -0000 @@ -76,8 +76,25 @@ if (pfxlen <= 1) rc = EINVAL; else { - int infd = open(filename, O_RDWR | O_CREAT, fmode); - + /* + * Open and lock the original file. We check the link count + * in order to avoid any link/unlink/rename race. + */ + int infd; + for (;;) { + struct stat st; + infd = open(filename, O_RDWR | O_CREAT | O_EXLOCK, fmode); + if (infd == -1) + break; + if (fstat(infd, &st) < 0) { + rc = errno; + close(infd); + return rc; + } + if (st.st_nlink == 1) + break; + close(infd); + } if (infd == -1) rc = errno; else { To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message