Make races between queue and runner impossible by implementing the policy:

1) queue never reads /queue.
2) queue writes to /queue only at message injection time.
3) runner does all reading, and all writing apart from 2).

ok gilles@

5 files changed, 32 insertions, 28 deletions

diff --git a/usr.sbin/smtpd/queue.c b/usr.sbin/smtpd/queue.c
index 19033c44d53..d0715dfa2a3 100644
--- a/usr.sbin/smtpd/queue.c
+++ b/usr.sbin/smtpd/queue.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: queue.c,v 1.50 2009/01/28 12:58:17 gilles Exp $	*/
+/*	$OpenBSD: queue.c,v 1.51 2009/01/28 17:29:11 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -306,7 +306,8 @@ queue_dispatch_mda(int sig, short event, void *p)
 		switch (imsg.hdr.type) {
 
 		case IMSG_QUEUE_MESSAGE_UPDATE: {
-			queue_message_update(imsg.data);
+			imsg_compose(env->sc_ibufs[PROC_RUNNER], IMSG_RUNNER_UPDATE_ENVELOPE,
+			    0, 0, -1, imsg.data, sizeof(struct message));
 			break;
 		}
 
@@ -369,7 +370,8 @@ queue_dispatch_mta(int sig, short event, void *p)
 		}
 
 		case IMSG_QUEUE_MESSAGE_UPDATE: {
-			queue_message_update(imsg.data);
+			imsg_compose(env->sc_ibufs[PROC_RUNNER], IMSG_RUNNER_UPDATE_ENVELOPE,
+			    0, 0, -1, imsg.data, sizeof(struct message));
 			break;
 		}
 
diff --git a/usr.sbin/smtpd/queue_shared.c b/usr.sbin/smtpd/queue_shared.c
index 14213225ba4..e80a619d085 100644
--- a/usr.sbin/smtpd/queue_shared.c
+++ b/usr.sbin/smtpd/queue_shared.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: queue_shared.c,v 1.3 2009/01/28 12:58:17 gilles Exp $	*/
+/*	$OpenBSD: queue_shared.c,v 1.4 2009/01/28 17:29:11 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -82,9 +82,7 @@ queue_delete_layout_message(char *queuepath, char *msgid)
 		fatalx("snprintf");
 
 	if (rename(rootdir, purgedir) == -1) {
-		if (errno == ENOENT)
-			return;
-		fatal("queue_delete_incoming_message: rename");
+		fatal("queue_delete_layout_message: rename");
 	}
 }
 
@@ -149,8 +147,7 @@ queue_remove_layout_envelope(char *queuepath, struct message *message)
 		fatal("queue_remove_incoming_envelope: snprintf");
 
 	if (unlink(pathname) == -1)
-		if (errno != ENOENT)
-			fatal("queue_remove_incoming_envelope: unlink");
+		fatal("queue_remove_incoming_envelope: unlink");
 
 	return 1;
 }
@@ -316,16 +313,19 @@ queue_delete_message(char *msgid)
 		fatal("queue_delete_message: snprintf");
 
 	if (unlink(msgpath) == -1)
-		if (errno != ENOENT)
-			fatal("queue_delete_message: unlink");
+		fatal("queue_delete_message: unlink");
 
-	if (rmdir(evpdir) == -1)
+	if (rmdir(evpdir) == -1) {
+		/* It is ok to fail rmdir with ENOENT here
+		 * because upon successful delivery of the
+		 * last envelope, we remove the directory.
+		 */
 		if (errno != ENOENT)
 			fatal("queue_delete_message: rmdir");
+	}
 
 	if (rmdir(rootdir) == -1)
-		if (errno != ENOENT)
-			fatal("queue_delete_message: rmdir");
+		fatal("#2 queue_delete_message: rmdir");
 
 	if (! bsnprintf(rootdir, MAXPATHLEN, "%s/%d", PATH_QUEUE,
 		hval))
@@ -433,8 +433,7 @@ queue_update_envelope(struct message *messagep)
 
 tempfail:
 	if (unlink(temp) == -1)
-		if (errno != ENOENT)
-			fatal("queue_update_envelope: unlink");
+		fatal("queue_update_envelope: unlink");
 	if (fp)
 		fclose(fp);
 
@@ -458,8 +457,6 @@ queue_load_envelope(struct message *messagep, char *evpid)
 
 	fp = fopen(pathname, "r");
 	if (fp == NULL) {
-		if (errno == ENOENT)
-			return 0;
 		if (errno == ENOSPC || errno == ENFILE)
 			return 0;
 		fatal("queue_load_envelope: fopen");
diff --git a/usr.sbin/smtpd/runner.c b/usr.sbin/smtpd/runner.c
index be3c4554614..a26063e3414 100644
--- a/usr.sbin/smtpd/runner.c
+++ b/usr.sbin/smtpd/runner.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: runner.c,v 1.24 2009/01/28 13:29:40 gilles Exp $	*/
+/*	$OpenBSD: runner.c,v 1.25 2009/01/28 17:29:11 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -188,6 +188,10 @@ runner_dispatch_queue(int sig, short event, void *p)
 			break;
 
 		switch (imsg.hdr.type) {
+		case IMSG_RUNNER_UPDATE_ENVELOPE: {
+			queue_message_update(imsg.data);
+			break;
+		}
 		default:
 			log_debug("runner_dispatch_queue: unexpected imsg %d",
 			    imsg.hdr.type);
@@ -498,7 +502,8 @@ runner_process_envelope(struct message *messagep)
 	runner_check_loop(messagep);
 
 	messagep->flags |= F_MESSAGE_SCHEDULED;
-	queue_update_envelope(messagep);
+	if (! queue_update_envelope(messagep))
+		return;
 
 	if (! bsnprintf(evppath, MAXPATHLEN, "%s/%d/%s%s/%s", PATH_QUEUE,
 		queue_hash(messagep->message_id), messagep->message_id,
@@ -555,7 +560,8 @@ runner_process_runqueue(struct smtpd *env)
 		message.lasttry = tm;
 		message.flags &= ~F_MESSAGE_SCHEDULED;
 		message.flags |= F_MESSAGE_PROCESSING;
-		queue_update_envelope(&message);
+		if (! queue_update_envelope(&message))
+			continue;
 
 		messagep = calloc(1, sizeof (struct message));
 		if (messagep == NULL)
@@ -667,11 +673,8 @@ again:
 	}
 
 recurse:
-	if (curdir == NULL) {
-		if (errno == ENOENT)
-			goto again;
+	if (curdir == NULL)
 		fatal("runner_envelope_next: opendir failed");
-	}
 	goto again;
 }
 
diff --git a/usr.sbin/smtpd/smtp_session.c b/usr.sbin/smtpd/smtp_session.c
index 32a9e4b3573..0298da84b77 100644
--- a/usr.sbin/smtpd/smtp_session.c
+++ b/usr.sbin/smtpd/smtp_session.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtp_session.c,v 1.41 2009/01/28 11:27:57 gilles Exp $	*/
+/*	$OpenBSD: smtp_session.c,v 1.42 2009/01/28 17:29:11 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -717,7 +717,7 @@ session_pickup(struct session *s, struct submit_status *ss)
 
 	case S_DONE:
 		s->s_state = S_HELO;
-
+		s->s_msg.message_id[0] = '\0';
 		session_respond(s, "250 %s Message accepted for delivery",
 		    s->s_msg.message_id);
 
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 81a152a4dae..d6d115444c4 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.51 2009/01/28 13:29:40 gilles Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.52 2009/01/28 17:29:11 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -186,6 +186,8 @@ enum imsg_type {
 	IMSG_QUEUE_MESSAGE_FD,
 	IMSG_QUEUE_MESSAGE_FILE,
 
+	IMSG_RUNNER_UPDATE_ENVELOPE,
+
 	IMSG_BATCH_CREATE,
 	IMSG_BATCH_APPEND,
 	IMSG_BATCH_CLOSE,