Date: Mon, 24 Jun 2002 23:00:06 -0700 (PDT) From: Paul Herman <pherman@frenchfries.net> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/38676 change request for pw command Message-ID: <200206250600.g5P606q08404@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/38676; it has been noted by GNATS. From: Paul Herman <pherman@frenchfries.net> To: FreeBSD-gnats-submit@FreeBSD.ORG Cc: Takumi ISHII <takishii@xephion.ne.jp>, "Geoffrey C. Speicher" <geoff@sea-incorporated.com>, "Matthew D. Fuller" <fullermd@over-yonder.net> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200206250600.g5P606q08404>