The ugly spectre of MBX mailbox corruption reared its ugly head again this
week, and I've tracked it down to a race condition the MBX locking
protocol. The issue is with the lockfile that is created in /tmp - if this
file is removed after a process has opened it but before that process has
acquired a lock, then there is the potential for a second process to
recreate the file and also acquire a lock. In our case, this was leading to
two copies of exim writing to a mailbox at the same time.
Both exim and UW imapd take steps to avoid this problem - exim by not
removing the temporary file if the mailbox itself is locked, and UW imapd
by checking after it locks the filehandle that this is still the same as
the file on disk. Unfortunately, these approaches are incompatable -
consider the following:
exim 1: Read-locks mailbox, Creates and opens temporary lockfile
imapd : Opens mailbox (this involves briefly locking the temporary lockfile,
the unlock code unlinks the temporary file if ther are no other
locks on it, even if there are locks on the mailbox itself)
exim 2: Read-locks mailbox, Creates, opens and write-locks temporary
lockfile, Starts writing message to mailbox
exim 1: Write locks the filehandle of the unlinked temporary lockfile,
writes message to mailbox. Does not unlink temporary lockfile as
exim 2 still has a lock on the mailbox itself.
exim 2: Writes the remainder of its message, unlinks the temporary
lockfile.
At this point, the mailbox is corrupted and imapd is unable to read it.
There's also the theoretical possibility of a race condition between
checking for locks on the mailbox and unlink the temporary lockfile,
although I've not seen this in practice (it was hard enough to reproduce
the situation outlined above).
The patch below simply adds the same check that IW imapd does - after locks
are aquired, it checks that fstating the filehandle and lstating the
lockfile itself return the same device/inode pair. This is added to both
the appendfile transport and exim_lock utility (also there's a fix for a
typo in the latter).
And before anyone says it, yes I'm planning to dump UW imapd and switch to
Courier and maildir as soon as circumstances allow.
-----BEGIN PATCH-----
diff -Naur exim-4.32/src/exim_lock.c exim-4.32-mbx_lock/src/exim_lock.c
--- exim-4.32/src/exim_lock.c 2004-04-15 09:27:01.000000000 +0100
+++ exim-4.32-mbx_lock/src/exim_lock.c 2004-04-22 15:34:29.000000000 +0100
@@ -308,7 +308,7 @@
for (j = 0; j < lock_retries; j++)
{
int sleep_before_retry = TRUE;
- struct stat statbuf;
+ struct stat statbuf, ostatbuf;
/* Try to build a lock file if so configured */
@@ -395,7 +395,7 @@
if (use_fcntl)
printf("exim_lock: fcntl() read lock successfully applied\n");
if (use_flock)
- printf("exim_lock: fcntl() read lock successfully applied\n");
+ printf("exim_lock: flock() read lock successfully applied\n");
}
}
else goto RETRY; /* Message already output */
@@ -451,7 +451,16 @@
printf("exim_lock: flock() lock successfully applied to mbx "
"lock file %s\n", tempname);
}
- break;
+
+ if (lstat(tempname, &statbuf) || fstat(md, &ostatbuf) ||
+ (statbuf.st_dev != ostatbuf.st_dev) ||
+ (statbuf.st_ino != ostatbuf.st_ino))
+ {
+ if (!quiet) printf("exim_lock: mbx lock file %s changed between "
+ "creation and locking\n", tempname);
+ goto RETRY;
+ }
+ else break;
}
else goto RETRY; /* Message already output */
}
diff -Naur exim-4.32/src/transports/appendfile.c exim-4.32-mbx_lock/src/transports/appendfile.c
--- exim-4.32/src/transports/appendfile.c 2004-04-15 09:27:01.000000000 +0100
+++ exim-4.32-mbx_lock/src/transports/appendfile.c 2004-04-22 15:46:41.000000000 +0100
@@ -1971,7 +1971,20 @@
if (apply_lock(mbx_lockfd, F_WRLCK, ob->use_fcntl,
ob->lock_fcntl_timeout, ob->use_flock, ob->lock_flock_timeout) >= 0)
- break;
+ {
+ struct stat ostatbuf;
+ if (lstat(mbx_lockname, &statbuf) ||
+ fstat(mbx_lockfd, &ostatbuf) ||
+ (statbuf.st_dev != ostatbuf.st_dev) ||
+ (statbuf.st_ino != ostatbuf.st_ino))
+ {
+ DEBUG(D_transport) debug_printf("MBX lockfile %s changed "
+ "between creation and locking\n", mbx_lockname);
+ close(mbx_lockfd);
+ mbx_lockfd = -1;
+ }
+ else break;
+ }
DEBUG(D_transport) debug_printf("failed to lock %s: %s\n", mbx_lockname,
strerror(errno));
-----END PATCH-----
mrj
--
Mark Rigby-Jones, Head of Systems, Community Internet plc
Windsor House, 12 High Street, Kidlington, Oxford OX5 2PJ, UK
Tel: +44-1865-856000 (Direct: +44-1865-856009) Fax: +44-1865-856001
mrj@??? <*> http://www.community.net.uk/~mrj
