phil@sequel.UUCP (06/02/83)
There is another bug in 4.1 /bin/mail. I just had my third
complaint of mail getting mixed together so decided to fix it
once and for all. I looked through my bug files and found only
the comsat problem (fwrite to mail but no flush or close before
writing to comsat). I had already fixed the comsat bug.
I suspected the locking protocol was fubar (was running setuid
root, etc). This proved not to be the case. I tested it by
running the following script:
(mail -s "Try number 1" phil < /etc/passwd ) &
(mail -s "Try number 2" phil < /etc/passwd ) &
(mail -s "Try number 3" phil < /etc/passwd ) &
(mail -s "Try number 4" phil < /etc/passwd ) &
(mail -s "Try number 5" phil < /etc/passwd ) &
I found that usually only 1 to 3 copies arrived usually in a
jumbled order. Humm. Must be failing to lock the mailbox right?
WRONG! I crawled through the routine lock(), and lock1(). They
appear to be 100% ok. In looking at where mail does the local
delivery I noticed the following sequence of code:
mask = umask(MAILMODE);
---> malf = fopen(file, "a");
umask(mask);
if (malf == NULL) {
fprintf(stdout, "mail: cannot append to %s\n", file);
return(0);
}
---> lock(file);
chown(file, pw->pw_uid, pw->pw_gid);
{
f = open("/dev/mail", 1);
sprintf(buf, "%s@%d\n", name, ftell(malf));
}
copylet(n, malf, ORDINARY);
fclose(malf); /* here so comsat works! */
if (f >= 0) {
write(f, buf, strlen(buf)+1);
close(f);
}
/* fclose(malf); not here bozo */
unlock();
return(1);
Notice how the file is opened for appending BEFORE being locked.
This presents a race condition where several mail's all open the
file for appending (all using the current length of the file)
then take turns locking the file overwriting each other.
The fix of course is to move the lock() above the fopen and add a
unlock() if you can't open the file.
---> lock(file); /* Bug fix. sequel!phil */
mask = umask(MAILMODE);
---> malf = fopen(file, "a");
umask(mask);
if (malf == NULL) {
unlock(); /* Bug fix. see above */
fprintf(stdout, "mail: cannot append to %s\n", file);
return(0);
}
/* lock(file); not early enough */
chown(file, pw->pw_uid, pw->pw_gid);
{
f = open("/dev/mail", 1);
sprintf(buf, "%s@%d\n", name, ftell(malf));
}
copylet(n, malf, ORDINARY);
fclose(malf); /* here so comsat works! */
if (f >= 0) {
write(f, buf, strlen(buf)+1);
close(f);
}
/* fclose(malf); not here bozo */
unlock();
return(1);
After this fix, my test ran 10 times without a fail. If you are
concerned about your mail you should fix this one. I don't know
if someone else already posted this bug but I have never seen it.
I don't have a copy of V7 /bin/mail handy but I suspect it exists
there also.
Phil Hochstetler
Sequel Computer Systems, Inc.
Portland, Oregon
(503) 626-5700
uucp: ogcvax!sequel!phil
pur-ee!sequel!phil