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

Daniel Roethlisberger daniel at roe.ch
Fri Jul 3 15:26:27 PDT 2009


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 your at email.com your_second at email.com < /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/
--- 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);




More information about the Submit mailing list