dma(8): Fix race condition in multi-recipient delivery

--NU0Ex4SbNnrxsi6C
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

When delivering mail to multiple recipients, dma(8) will fork for
each recipient and attempt parallel delivery.  Separate delivery
processes for multiple recipients of the same message all use the
same FILE* on the same file descriptor.  If a process is loosing
the CPU during the delivery process, another delivery process for
the same message will wreak havoc on the file descriptor.  The
symptom is dma(8) logging a failed delivery and bouncing the
message for some or all of the recipients:

Jul  3 20:57:21 marvin dma[32640]: 15c.284010d0: remote delivery
failed:corrupted queue file

Messages may also silently corrupt without dma(8) even noticing.

To reproduce:

$ dd if=/dev/urandom bs=1k count=100 | openssl base64 > /tmp/data
$ mail -s test [e-mail] [e-mail] < /tmp/data

Then compare what you received with /tmp/data.

The attached patch resolves the race condition by locking the
queue file during delivery.

Note that this is just a quick fix against message corruption.
I'm suspecting that a better fix would be to rewrite dma(8) to
fork a process per message, not per recipient, OR copy the spool
file per recipient and fork per spool file.

Disclaimer: Tested and fixed on FreeBSD 7.2, not DragonFlyBSD.

-- 
Daniel Roethlisberger
http://daniel.roe.ch/

--NU0Ex4SbNnrxsi6C
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="dma-lock-delivery.diff"

--- libexec/dma/dma.c.orig	2009-07-03 21:56:03.000000000 +0200
+++ libexec/dma/dma.c	2009-07-04 00:12:53.000000000 +0200
@@ -612,6 +612,7 @@
 	const char *errmsg = "unknown bounce reason";
 	struct timeval now;
 	struct stat st;
+	struct flock fl;
 
 	syslog(LOG_INFO, "%s: mail from=<%s> to=<%s>",
 	       it->queueid, it->sender, it->addr);
@@ -620,11 +621,27 @@
 	syslog(LOG_INFO, "%s: trying delivery",
 	       it->queueid);
 
+	bzero(&fl, sizeof(fl));
+	fl.l_type = F_WRLCK;
+	fl.l_whence = SEEK_SET;
+	if (fcntl(fileno(it->queuef), F_SETLKW, &fl) == -1) {
+		syslog(LOG_ERR, "%s: failed to lock queue file: %m",
+				it->queueid);
+	}
+
 	if (it->remote)
 		error = deliver_remote(it, &errmsg);
 	else
 		error = deliver_local(it, &errmsg);
 
+	bzero(&fl, sizeof(fl));
+	fl.l_type = F_UNLCK;
+	fl.l_whence = SEEK_SET;
+	if (fcntl(fileno(it->queuef), F_SETLKW, &fl) == -1) {
+		syslog(LOG_ERR, "%s: failed to unlock queue file: %m",
+				it->queueid);
+	}
+
 	switch (error) {
 	case 0:
 		unlink(it->queuefn);

--NU0Ex4SbNnrxsi6C--