DragonFly submit List (threaded) for 2009-07
DragonFly BSD
DragonFly submit List (threaded) for 2009-07
[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]

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


From: Daniel Roethlisberger <daniel@xxxxxx>
Date: Sat, 4 Jul 2009 00:23:36 +0200
Mail-followup-to: Daniel Roethlisberger <daniel@roe.ch>, submit@dragonflybsd.org

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@email.com your_second@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);


[Date Prev][Date Next]  [Thread Prev][Thread Next]  [Date Index][Thread Index]