Date: Thu, 18 Jul 1996 16:42:43 -0500 From: Travis Hassloch x231 <travis@EvTech.com> To: bugtraq@netspace.org Cc: freebsd-current@freebsd.org, current-users@NetBSD.ORG, cross@math.psu.edu Subject: Re: BSD mail.local has race condition - fix Message-ID: <199607182142.QAA19123@tahiti.evtech.com>
next in thread | raw e-mail | index | archive | help
This fix is relative to: static char sccsid[] = "from: @(#)mail.local.c 5.6 (Berkeley) 6/19/91"; as modified into: "$Id: mail.local.c,v 1.1.1.1 1995/10/18 08:43:19 deraadt Exp $" The problem I described does _not_ occur in BSD/OS. I have cc'ed everyone I think might want to integrate it ('cept OpenBSD, Theo already has a copy); the rest is up to you. I believe it to fix the problem but have not even compiled it as I lack a BSD environment here. --- mail.local.c~ Tue Jul 16 19:00:07 1996 +++ mail.local.c Tue Jul 16 21:02:15 1996 @@ -168,9 +168,9 @@ char *name; int lockfile; { - struct stat sb; + struct stat sb, fsb; struct passwd *pw; - int created, mbfd, nr, nw, off, rval=0, lfd=-1; + int mbfd=-1, nr, nw, off, rval=1, lfd=-1; char biffmsg[100], buf[8*1024], path[MAXPATHLEN], lpath[MAXPATHLEN]; off_t curoff; @@ -194,59 +194,94 @@ return(1); } } - - if (!(created = lstat(path, &sb)) && - (sb.st_nlink != 1 || S_ISLNK(sb.st_mode))) { - err(NOTFATAL, "%s: linked file", path); - return(1); - } - if((mbfd = open(path, O_APPEND|O_WRONLY|O_EXLOCK, - S_IRUSR|S_IWUSR)) < 0) { + /* after this point, always exit via bad to remove lockfile */ +retry: + if (lstat(path, &sb)) { + if (errno != ENOENT) { + err(NOTFATAL, "%s: %s", path, strerror(errno)); + goto bad; + } if ((mbfd = open(path, O_APPEND|O_CREAT|O_WRONLY|O_EXLOCK, - S_IRUSR|S_IWUSR)) < 0) { - err(NOTFATAL, "%s: %s", path, strerror(errno)); - return(1); + S_IRUSR|S_IWUSR)) < 0) { + if (errno == EEXIST) { + /* file appeared since lstat */ + goto retry; + } + else { + err(NOTFATAL, "%s: %s", path, strerror(errno)); + goto bad; + } + } + /* + * Set the owner and group. Historically, binmail repeated this at + * each mail delivery. We no longer do this, assuming that if the + * ownership or permissions were changed there was a reason for doing + * so. + */ + if (fchown(mbfd, pw->pw_uid, pw->pw_gid) < 0) { + err(NOTFATAL, "chown %u:%u: %s", + pw->pw_uid, pw->pw_gid, name); + goto bad; + } } + else { + if (sb.st_nlink != 1 || S_ISLNK(sb.st_mode)) { + err(NOTFATAL, "%s: linked file", path); + goto bad; + } + if ((mbfd = open(path, O_APPEND|O_WRONLY|O_EXLOCK, + S_IRUSR|S_IWUSR)) < 0) { + err(NOTFATAL, "%s: %s", path, strerror(errno)); + goto bad; + } + if (fstat(mbfd, &fsb)) { + /* relating error to path may be bad style */ + err(NOTFATAL, "%s: %s", path, strerror(errno)); + goto bad; + } + if (sb.st_dev != fsb.st_dev || sb.st_ino != fsb.st_ino) { + err(NOTFATAL, "%s: changed after open", path); + goto bad; + } + /* paranoia? */ + if (fsb.st_nlink != 1 || S_ISLNK(fsb.st_mode)) { + err(NOTFATAL, "%s: linked file", path); + goto bad; + } } curoff = lseek(mbfd, 0, SEEK_END); (void)sprintf(biffmsg, "%s@%qd\n", name, curoff); if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { err(FATAL, "temporary file: %s", strerror(errno)); - rval = 1; goto bad; } + /* This section (to trunc) is ugly */ while ((nr = read(fd, buf, sizeof(buf))) > 0) for (off = 0; off < nr; off += nw) if ((nw = write(mbfd, buf + off, nr - off)) < 0) { err(NOTFATAL, "%s: %s", path, strerror(errno)); goto trunc; } - if (nr < 0) { + if (!nr) { + rval = 0; + } + else { err(FATAL, "temporary file: %s", strerror(errno)); trunc: (void)ftruncate(mbfd, curoff); - rval = 1; } - /* - * Set the owner and group. Historically, binmail repeated this at - * each mail delivery. We no longer do this, assuming that if the - * ownership or permissions were changed there was a reason for doing - * so. - */ bad: - if(lockfile) { - if(lfd >= 0) { - unlink(lpath); - close(lfd); - } + if(lfd >= 0) { + unlink(lpath); + close(lfd); } - if (created) - (void)fchown(mbfd, pw->pw_uid, pw->pw_gid); - (void)fsync(mbfd); /* Don't wait for update. */ - (void)close(mbfd); /* Implicit unlock. */ + if (mbfd >= 0) { + (void)fsync(mbfd); /* Don't wait for update. */ + (void)close(mbfd); /* Implicit unlock. */ + } if (!rval) notifybiff(biffmsg); -- Travis Hassloch, Electronic Blacksmith | P=NP if (P=0 or N=1) There is a fine line between an email message and it's signature.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199607182142.QAA19123>