Major mda update:

- Fix: check external mda / mail.local exit code.
- Fix: check maildir rename(2) return code.
- Fix: check read(2) and write(2) return codes.
- Fix: in parent, batchp->env was not set to the env of the current process.
- Fix: clean file in tmp if maildir delivery fails.
- Fix: mark message as temporarily failed upon start, unmark upon sucessful
delivery. (safe default)
- Fix: kill all message drops, aka. PERMFAILUREs, with one exception: when the
local user no longer exists.
- Cleanup: store.c is merged with its only user, mda.c
- Feature: in parent, child_add now returns pointer to the new child struct.
This is used to store and later access child->mda_batch member in order to
associate children with their batches.
- Feature: in parent, external mda / mail.local will timeout after 5 minutes.

4 files changed, 329 insertions, 265 deletions

diff --git a/usr.sbin/smtpd/mda.c b/usr.sbin/smtpd/mda.c
index 57b823a61f4..2762180ecf5 100644
--- a/usr.sbin/smtpd/mda.c
+++ b/usr.sbin/smtpd/mda.c
@@ -1,8 +1,9 @@
-/*	$OpenBSD: mda.c,v 1.27 2009/09/03 08:19:13 jacekm Exp $	*/
+/*	$OpenBSD: mda.c,v 1.28 2009/09/04 11:49:23 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
  * Copyright (c) 2008 Pierre-Yves Ritschard <pyr@openbsd.org>
+ * Copyright (c) 2009 Jacek Masiulaniec <jacekm@dobremiasto.net>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -40,7 +41,8 @@ void		mda_dispatch_queue(int, short, void *);
 void		mda_dispatch_runner(int, short, void *);
 void		mda_setup_events(struct smtpd *);
 void		mda_disable_events(struct smtpd *);
-void		mda_remove_message(struct smtpd *, struct batch *, struct message *x);
+int		mda_store(struct batch *);
+void		mda_done(struct smtpd *, struct batch *);
 
 void
 mda_sig_handler(int sig, short event, void *p)
@@ -90,88 +92,90 @@ mda_dispatch_parent(int sig, short event, void *p)
 			break;
 
 		switch (imsg.hdr.type) {
-		case IMSG_MDA_MAILBOX_FILE: {
-			struct batch	*batchp = imsg.data;
-			struct session	*s;
-			struct message	*messagep;
-			enum message_status status;
-
-			IMSG_SIZE_CHECK(batchp);
-
-			messagep = &batchp->message;
-			status = messagep->status;
-
-			batchp = batch_by_id(env, batchp->id);
-			if (batchp == NULL)
-				fatalx("mda_dispatch_parent: internal inconsistency.");
-			s = batchp->sessionp;
-
-			messagep = message_by_id(env, batchp, messagep->id);
-			if (messagep == NULL)
-				fatalx("mda_dispatch_parent: internal inconsistency.");
-			messagep->status = status;
-
-			s->mboxfd = imsg.fd;
-			if (s->mboxfd == -1) {
-				mda_remove_message(env, batchp, messagep);
-				break;
-			}
+		case IMSG_PARENT_MAILBOX_OPEN: {
+			struct batch		*b = imsg.data;
+
+			IMSG_SIZE_CHECK(b);
+
+			if ((b = batch_by_id(env, b->id)) == NULL)
+				fatalx("mda: internal inconsistency");
+
+			/* parent ensures mboxfd is valid */
+			if (imsg.fd == -1)
+				fatalx("mda: mboxfd pass failure");
 
-			batchp->message = *messagep;
+			/* got user's mbox fd */
+			b->mboxfd = imsg.fd;
+
+			/* 
+			 * From now on, delivery session must be deinited in
+			 * the parent process as well as in mda.
+			 */
+			b->cleanup_parent = 1;
+
+			/* get message content fd */
 			imsg_compose_event(env->sc_ievs[PROC_PARENT],
-			    IMSG_PARENT_MESSAGE_OPEN, 0, 0, -1, batchp,
-			    sizeof(struct batch));
+			    IMSG_PARENT_MESSAGE_OPEN, 0, 0, -1, b,
+			    sizeof(*b));
 			break;
 		}
 
-		case IMSG_MDA_MESSAGE_FILE: {
-			struct batch	*batchp = imsg.data;
-			struct session	*s;
-			struct message	*messagep;
-			enum message_status status;
-
-			IMSG_SIZE_CHECK(batchp);
+		case IMSG_PARENT_MESSAGE_OPEN: {
+			struct batch	*b = imsg.data;
 
-			messagep = &batchp->message;
-			status = messagep->status;
+			IMSG_SIZE_CHECK(b);
 
-			batchp = batch_by_id(env, batchp->id);
-			if (batchp == NULL)
-				fatalx("mda_dispatch_parent: internal inconsistency.");
-			s = batchp->sessionp;
+			if ((b = batch_by_id(env, b->id)) == NULL)
+				fatalx("mda: internal inconsistency");
 
-			messagep = message_by_id(env, batchp, messagep->id);
-			if (messagep == NULL)
-				fatalx("mda_dispatch_parent: internal inconsistency.");
-			messagep->status = status;
-
-			s->messagefd = imsg.fd;
-			if (s->messagefd == -1) {
-				if (s->mboxfd != -1)
-					close(s->mboxfd);
-				mda_remove_message(env, batchp, messagep);
+			if (imsg.fd == -1) {
+				mda_done(env, b);
 				break;
 			}
 
-			if (store_message(batchp, messagep)) {
-				if (batchp->message.recipient.rule.r_action == A_MAILDIR)
-					imsg_compose_event(env->sc_ievs[PROC_PARENT],
-					    IMSG_PARENT_MAILBOX_RENAME, 0, 0, -1, batchp,
-					    sizeof(struct batch));
-			} else {
-				/* XXX: remove junk from tmp/ in mdir case */
+			b->datafd = imsg.fd;
+
+			/* got message content, copy it to mbox */
+			if (! mda_store(b)) {
 				env->stats->mda.write_error++;
-				messagep->status |= S_MESSAGE_TEMPFAILURE;
+				mda_done(env, b);
+				break;
 			}
+			close(b->datafd);
+			b->datafd = -1;
+
+			/* closing mboxfd will trigger EOF in forked mda */
+			fsync(b->mboxfd);
+			close(b->mboxfd);
+			b->mboxfd = -1;
+
+			/* ... unless it is maildir, in which case we need to
+			 * "trigger EOF" differently */
+			if (b->message.recipient.rule.r_action == A_MAILDIR)
+				imsg_compose_event(env->sc_ievs[PROC_PARENT],
+				    IMSG_PARENT_MAILDIR_RENAME, 0, 0, -1, b,
+				    sizeof(*b));
+			
+			/* Waiting for IMSG_MDA_FINALIZE... */
+			b->cleanup_parent = 0;
+			break;
+		}
+
+		case IMSG_MDA_FINALIZE: {
+			struct batch		*b = imsg.data;
+			enum message_status	 status;
+
+			IMSG_SIZE_CHECK(b);
 
-			if (s->mboxfd != -1)
-				close(s->mboxfd);
-			if (s->messagefd != -1)
-				close(s->messagefd);
+			status = b->message.status;
+			if ((b = batch_by_id(env, b->id)) == NULL)
+				fatalx("mda: internal inconsistency");
+			b->message.status = status;
 
-			mda_remove_message(env, batchp, messagep);
+			mda_done(env, b);
 			break;
 		}
+
 		default:
 			log_warnx("mda_dispatch_parent: got imsg %d",
 			    imsg.hdr.type);
@@ -263,78 +267,52 @@ mda_dispatch_runner(int sig, short event, void *p)
 
 		switch (imsg.hdr.type) {
 		case IMSG_BATCH_CREATE: {
-			struct batch *request = imsg.data;
-			struct batch *batchp;
-			struct session *s;
+			struct batch	*req = imsg.data;
+			struct batch	*b;
 
-			IMSG_SIZE_CHECK(request);
+			IMSG_SIZE_CHECK(req);
 
-			/* create a client session */
-			if ((s = calloc(1, sizeof(*s))) == NULL)
+			/* runner opens delivery session */
+			if ((b = malloc(sizeof(*b))) == NULL)
 				fatal(NULL);
-			s->s_state = S_INIT;
-			s->s_env = env;
-			s->s_id = queue_generate_id();
-			SPLAY_INSERT(sessiontree, &s->s_env->sc_sessions, s);
-
-			/* create the batch for this session */
-			batchp = calloc(1, sizeof (struct batch));
-			if (batchp == NULL)
-				fatal("mda_dispatch_runner: calloc");
-
-			*batchp = *request;
-			batchp->env = env;
-			batchp->sessionp = s;
-
-			s->batch = batchp;
-
-			TAILQ_INIT(&batchp->messages);
-			SPLAY_INSERT(batchtree, &env->batch_queue, batchp);
+			*b = *req;
+			b->env = env;
+			b->mboxfd = -1;
+			b->datafd = -1;
+			SPLAY_INSERT(batchtree, &env->batch_queue, b);
 			break;
 		}
 
 		case IMSG_BATCH_APPEND: {
 			struct message	*append = imsg.data;
-			struct message	*messagep;
-			struct batch	*batchp;
+			struct batch	*b;
 
 			IMSG_SIZE_CHECK(append);
 
-			messagep = calloc(1, sizeof (struct message));
-			if (messagep == NULL)
-				fatal("mda_dispatch_runner: calloc");
-
-			*messagep = *append;
-
-			batchp = batch_by_id(env, messagep->batch_id);
-			if (batchp == NULL)
-				fatalx("mda_dispatch_runner: internal inconsistency.");
+			/* runner submits the message to deliver */
+			if ((b = batch_by_id(env, append->batch_id)) == NULL)
+				fatalx("mda: internal inconsistency");
+			if (b->message.message_id[0])
+				fatal("mda: runner submitted extra msg");
+			b->message = *append;
 
- 			TAILQ_INSERT_TAIL(&batchp->messages, messagep, entry);
+			/* safe default */
+			b->message.status = S_MESSAGE_TEMPFAILURE;
 			break;
 		}
 
 		case IMSG_BATCH_CLOSE: {
-			struct batch	*batchp = imsg.data;
-			struct session	*s;
-			struct batch	 lookup;
-			struct message	*messagep;
+			struct batch	*b = imsg.data;
 
-			IMSG_SIZE_CHECK(batchp);
+			IMSG_SIZE_CHECK(b);
 
-			batchp = batch_by_id(env, batchp->id);
-			if (batchp == NULL)
-				fatalx("mda_dispatch_runner: internal inconsistency.");
-
-			s = batchp->sessionp;
-
-			lookup = *batchp;
-			TAILQ_FOREACH(messagep, &batchp->messages, entry) {
-				lookup.message = *messagep;
-				imsg_compose_event(env->sc_ievs[PROC_PARENT],
-				    IMSG_PARENT_MAILBOX_OPEN, 0, 0, -1, &lookup,
-				    sizeof(struct batch));
-			}
+			/* runner finished opening delivery session;
+			 * request user's mbox fd */
+			if ((b = batch_by_id(env, b->id)) == NULL)
+				fatalx("mda: internal inconsistency");
+			imsg_compose_event(env->sc_ievs[PROC_PARENT],
+			    IMSG_PARENT_MAILBOX_OPEN, 0, 0, -1, b,
+			    sizeof(*b));
 			break;
 		}
 		default:
@@ -433,23 +411,95 @@ mda(struct smtpd *env)
 	return (0);
 }
 
-void
-mda_remove_message(struct smtpd *env, struct batch *batchp, struct message *messagep)
+int
+mda_store(struct batch *b)
 {
-	imsg_compose_event(env->sc_ievs[PROC_QUEUE], IMSG_QUEUE_MESSAGE_UPDATE, 0, 0,
-	    -1, messagep, sizeof (struct message));
-
-	if ((messagep->status & S_MESSAGE_TEMPFAILURE) == 0 &&
-	    (messagep->status & S_MESSAGE_PERMFAILURE) == 0) {
-		log_info("%s: to=<%s@%s>, delay=%d, stat=Sent",
-		    messagep->message_uid,
-		    messagep->recipient.user,
-		    messagep->recipient.domain,
-		    time(NULL) - messagep->creation);
-	}
+	FILE	 *dst;
+	FILE	 *src;
+	int	  ch;
+ 
+	if ((dst = fdopen(b->mboxfd, "w")) == NULL)
+		return 0;
+	if ((src = fdopen(b->datafd, "r")) == NULL)
+		return 0;
+	 
+	/* add Delivered-To to help loop detection */
+	fprintf(dst, "Delivered-To: %s@%s\n",
+	    b->message.session_rcpt.user,
+	    b->message.session_rcpt.domain);
+
+	/* write message data */
+	/* XXX: it blocks in !mdir case */
+	while ((ch = fgetc(src)) != EOF)
+		if (fputc(ch, dst) == EOF)
+			break;
+	if (ferror(src) || fflush(dst) || ferror(dst))
+		return 0;
+	
+	return 1;
+}
 
-	SPLAY_REMOVE(sessiontree, &env->sc_sessions, batchp->sessionp);
-	free(batchp->sessionp);
+void
+mda_done(struct smtpd *env, struct batch *b)
+{
+	if (b->cleanup_parent) {
+		/*
+		 * Error has occured while both parent and mda maintain some
+		 * state for this delivery session.  Need to deinit both.
+		 * Deinit parent first.
+		 */
+
+		if (b->message.recipient.rule.r_action == A_MAILDIR) {
+			/*
+			 * In case of maildir, deiniting parent's state consists
+			 * of removing the file in tmp.
+			 */
+			imsg_compose_event(env->sc_ievs[PROC_PARENT],
+			    IMSG_PARENT_MAILDIR_FAIL, 0, 0, -1, b,
+			    sizeof(*b));
+		} else {
+			/*
+			 * In all other cases, ie. mbox and external, deiniting
+			 * parent's state consists of killing its child, and
+			 * freeing associated struct child.
+			 *
+			 * Requesting that parent does this cleanup involves
+			 * racing issues.  The race-free way is to simply wait.
+			 * Eventually, child timeout in parent will be hit.
+			 */
+		}
 
-	queue_remove_batch_message(env, batchp, messagep);
+		/*
+		 * Either way, parent will eventually send IMSG_MDA_FINALIZE.
+		 * Then, the mda deinit will happen.
+		 */
+		b->cleanup_parent = 0;
+	} else {
+		/*
+		 * Deinit mda.
+		 */
+
+		/* update runner (currently: via queue) */
+		imsg_compose_event(env->sc_ievs[PROC_QUEUE],
+		    IMSG_QUEUE_MESSAGE_UPDATE, 0, 0, -1,
+		    &b->message, sizeof(b->message));
+
+		/* log status */
+		if (b->message.status & S_MESSAGE_PERMFAILURE)
+			log_debug("mda: permanent failure");
+		else if (b->message.status & S_MESSAGE_TEMPFAILURE)
+			log_debug("mda: temporary failure");
+		else
+			log_info("%s: to=<%s@%s>, delay=%d, stat=Sent",
+			    b->message.message_uid,
+			    b->message.recipient.user,
+			    b->message.recipient.domain,
+			    time(NULL) - b->message.creation);
+
+		/* deallocate resources */
+		SPLAY_REMOVE(batchtree, &env->batch_queue, b);
+		close(b->mboxfd);
+		close(b->datafd);
+		free(b);
+	}
 }
diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c
index 94e31feb315..184642a62c0 100644
--- a/usr.sbin/smtpd/smtpd.c
+++ b/usr.sbin/smtpd/smtpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.c,v 1.84 2009/09/03 08:19:13 jacekm Exp $	*/
+/*	$OpenBSD: smtpd.c,v 1.85 2009/09/04 11:49:23 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -65,7 +65,6 @@ void		 parent_sig_handler(int, short, void *);
 int		 parent_open_message_file(struct batch *);
 int		 parent_mailbox_open(char *, struct passwd *, struct batch *);
 int		 parent_filename_open(char *, struct passwd *, struct batch *);
-int		 parent_mailfile_rename(struct batch *, struct path *);
 int		 parent_maildir_open(char *, struct passwd *, struct batch *);
 int		 parent_maildir_init(struct passwd *, char *);
 int		 parent_external_mda(char *, struct passwd *, struct batch *);
@@ -76,10 +75,14 @@ int		 path_starts_with(char *, char *);
 
 void		 fork_peers(struct smtpd *);
 
-void		 child_add(struct smtpd *, pid_t, int, int);
+struct child	*child_add(struct smtpd *, pid_t, int, int);
 void		 child_del(struct smtpd *, pid_t);
 struct child	*child_lookup(struct smtpd *, pid_t);
 
+void		 parent_mda_permfail(struct smtpd *, struct batch *);
+void		 parent_mda_tempfail(struct smtpd *, struct batch *);
+void		 parent_mda_success(struct smtpd *, struct batch *);
+
 extern char	**environ;
 
 int __b64_pton(char const *, unsigned char *, size_t);
@@ -385,7 +388,7 @@ parent_dispatch_mta(int fd, short event, void *p)
 }
 
 void
-parent_dispatch_mda(int fd, short event, void *p)
+parent_dispatch_mda(int imsgfd, short event, void *p)
 {
 	struct smtpd		*env = p;
 	struct imsgev		*iev;
@@ -426,7 +429,7 @@ parent_dispatch_mda(int fd, short event, void *p)
 			char *pw_name;
 			char *file;
 			u_int8_t i;
-			int desc;
+			int fd;
 			struct action_handler {
 				enum action_type action;
 				int (*handler)(char *, struct passwd *, struct batch *);
@@ -439,6 +442,7 @@ parent_dispatch_mda(int fd, short event, void *p)
 
 			IMSG_SIZE_CHECK(batchp);
 
+			batchp->env = env;
 			path = &batchp->message.recipient;
 			if (batchp->type & T_BOUNCE_BATCH) {
 				path = &batchp->message.sender;
@@ -461,43 +465,48 @@ parent_dispatch_mda(int fd, short event, void *p)
 			pw = getpwnam(pw_name);
 			if (pw == NULL) {
 				if (errno)
-					batchp->message.status |= S_MESSAGE_TEMPFAILURE;
+					parent_mda_tempfail(env, batchp);
 				else
-					batchp->message.status |= S_MESSAGE_PERMFAILURE;
-				imsg_compose_event(iev, IMSG_MDA_MAILBOX_FILE, 0, 0,
-				    -1, batchp, sizeof(struct batch));
+					parent_mda_permfail(env, batchp);
 				break;
 			}
 
 			if (setegid(pw->pw_gid) || seteuid(pw->pw_uid))
 				fatal("privdrop failed");
 
-			desc = action_hdl_table[i].handler(file, pw, batchp);
-			imsg_compose_event(iev, IMSG_MDA_MAILBOX_FILE, 0, 0,
-			    desc, batchp, sizeof(struct batch));
+			fd = action_hdl_table[i].handler(file, pw, batchp);
 
 			if (setegid(0) || seteuid(0))
-				fatal("privdrop failed");
+				fatal("privraise failed");
 
+			if (fd == -1)
+				parent_mda_tempfail(env, batchp);
+			else
+				imsg_compose_event(iev,
+				    IMSG_PARENT_MAILBOX_OPEN, 0, 0, fd, batchp,
+				    sizeof(*batchp));
 			break;
 		}
 		case IMSG_PARENT_MESSAGE_OPEN: {
 			struct batch *batchp = imsg.data;
-			int desc;
+			int fd;
 
 			IMSG_SIZE_CHECK(batchp);
 
-			desc = parent_open_message_file(batchp);
+			fd = parent_open_message_file(batchp);
 
-			imsg_compose_event(iev, IMSG_MDA_MESSAGE_FILE, 0, 0,
-			    desc, batchp, sizeof(struct batch));
+			imsg_compose_event(iev, IMSG_PARENT_MESSAGE_OPEN,
+			    0, 0, fd, batchp, sizeof(struct batch));
 
 			break;
 		}
-		case IMSG_PARENT_MAILBOX_RENAME: {
-			struct batch *batchp = imsg.data;
-			struct path *path;
-			struct passwd *pw;
+		case IMSG_PARENT_MAILDIR_FAIL:
+		case IMSG_PARENT_MAILDIR_RENAME: {
+			char		 tmp[MAXPATHLEN], new[MAXPATHLEN];
+			struct batch	*batchp = imsg.data;
+			struct path	*path;
+			struct passwd	*pw;
+			int		 ret;
 
 			IMSG_SIZE_CHECK(batchp);
 
@@ -506,18 +515,39 @@ parent_dispatch_mda(int fd, short event, void *p)
 				path = &batchp->message.sender;
 			}
 
+			errno = 0;
 			pw = getpwnam(path->pw_name);
-			if (pw == NULL)
-				break;			
+			if (pw == NULL) {
+				if (errno)
+					parent_mda_tempfail(env, batchp);
+				else
+					parent_mda_permfail(env, batchp);
+				break;
+			}
+
+			if (! bsnprintf(tmp, sizeof(tmp), "%s/tmp/%s",
+				path->rule.r_value.path, batchp->message.message_uid))
+				fatal("parent_dispatch_mda: snprintf");
+			if (! bsnprintf(new, sizeof(new), "%s/new/%s",
+				path->rule.r_value.path, batchp->message.message_uid))
+				fatal("parent_dispatch_mda: snprintf");
 
 			if (seteuid(pw->pw_uid) == -1)
 				fatal("privdrop failed");
 
-			parent_mailfile_rename(batchp, path);
+			if (imsg.hdr.type == IMSG_PARENT_MAILDIR_FAIL)
+				ret = -1;
+			else
+				ret = rename(tmp, new);
 
 			if (seteuid(0) == -1)
 				fatal("privraise failed");
 
+			if (ret < 0) {
+				unlink(tmp);
+				parent_mda_tempfail(env, batchp);
+			} else
+				parent_mda_success(env, batchp);
 			break;
 		}
 		default:
@@ -757,10 +787,18 @@ parent_sig_handler(int sig, short event, void *p)
 				break;
 
 			case CHILD_MDA:
-				if (fail)
+				if (WIFSIGNALED(status) &&
+				    WTERMSIG(status) == SIGALRM) {
+					free(cause);
+					asprintf(&cause, "terminated; timeout");
+				}
+				if (fail) {
 					log_warnx("external mda %s", cause);
-				else
+					parent_mda_tempfail(env, &child->mda_batch);
+				} else {
 					log_debug("external mda %s", cause);
+					parent_mda_success(env, &child->mda_batch);
+				}
 				break;
 
 			case CHILD_ENQUEUE_OFFLINE:
@@ -779,7 +817,6 @@ parent_sig_handler(int sig, short event, void *p)
 			}
 
 			child_del(env, child->pid);
-
 			free(cause);
 		} while (pid > 0 || (pid == -1 && errno == EINTR));
 
@@ -964,7 +1001,7 @@ fork_peers(struct smtpd *env)
 	child_add(env, smtp(env), CHILD_DAEMON, PROC_SMTP);
 }
 
-void
+struct child *
 child_add(struct smtpd *env, pid_t pid, int type, int title)
 {
 	struct child	*child;
@@ -978,6 +1015,8 @@ child_add(struct smtpd *env, pid_t pid, int type, int title)
 
 	if (SPLAY_INSERT(childtree, &env->children, child) != NULL)
 		fatalx("child_add: double insert");
+
+	return (child);
 }
 
 void
@@ -1175,10 +1214,8 @@ parent_open_message_file(struct batch *batchp)
 	hval = queue_hash(messagep->message_id);
 
 	if (! bsnprintf(pathname, sizeof(pathname), "%s%s/%d/%s/message",
-		PATH_SPOOL, PATH_QUEUE, hval, batchp->message_id)) {
-		batchp->message.status |= S_MESSAGE_PERMFAILURE;
-		return -1;
-	}
+		PATH_SPOOL, PATH_QUEUE, hval, batchp->message_id))
+		fatal("parent_open_message_file: snprintf");
 
 	fd = open(pathname, O_RDONLY);
 	return fd;
@@ -1189,21 +1226,18 @@ parent_mailbox_open(char *path, struct passwd *pw, struct batch *batchp)
 {
 	pid_t pid;
 	int pipefd[2];
+	struct child *child;
 	char sender[MAX_PATH_SIZE];
 
 	/* This can never happen, but better safe than sorry. */
 	if (! bsnprintf(sender, MAX_PATH_SIZE, "%s@%s",
 		batchp->message.sender.user,
-		batchp->message.sender.domain)) {
-		batchp->message.status |= S_MESSAGE_PERMFAILURE;
-		return -1;
-	}
+		batchp->message.sender.domain))
+		fatal("parent_mailbox_open: bogus email length");
 
 	log_debug("executing mail.local");
-	if (socketpair(AF_UNIX, SOCK_STREAM, 0, pipefd) == -1) {
-		batchp->message.status |= S_MESSAGE_PERMFAILURE;
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, pipefd) == -1)
 		return -1;
-	}
 
 	/* raise privileges because mail.local needs root to
 	 * deliver to user mailboxes.
@@ -1215,7 +1249,6 @@ parent_mailbox_open(char *path, struct passwd *pw, struct batch *batchp)
 	if (pid == -1) {
 		close(pipefd[0]);
 		close(pipefd[1]);
-		batchp->message.status |= S_MESSAGE_PERMFAILURE;
 		return -1;
 	}
 
@@ -1225,6 +1258,9 @@ parent_mailbox_open(char *path, struct passwd *pw, struct batch *batchp)
 		close(STDERR_FILENO);
 		dup2(pipefd[1], 0);
 
+		/* avoid hangs by setting a 5m timeout */
+		alarm(300);
+
 		execlp(PATH_MAILLOCAL, "mail.local", "-f", sender, pw->pw_name,
 		    (void *)NULL);
 		_exit(1);
@@ -1233,7 +1269,10 @@ parent_mailbox_open(char *path, struct passwd *pw, struct batch *batchp)
 	if (seteuid(pw->pw_uid) == -1)
 		fatal("privdrop failed");
 
-	child_add(batchp->env, pid, CHILD_MDA, -1);
+	child = child_add(batchp->env, pid, CHILD_MDA, -1);
+
+	/* Each child relates to a batch; record this relationship. */
+	child->mda_batch = *batchp;
 
 	close(pipefd[1]);
 	return pipefd[0];
@@ -1261,55 +1300,23 @@ parent_maildir_init(struct passwd *pw, char *root)
 int
 parent_maildir_open(char *path, struct passwd *pw, struct batch *batchp)
 {
-	int fd;
 	char tmp[MAXPATHLEN];
 	int mode = O_CREAT|O_RDWR|O_TRUNC|O_SYNC;
 
-	if (! parent_maildir_init(pw, path)) {
-		batchp->message.status |= S_MESSAGE_TEMPFAILURE;
+	if (! parent_maildir_init(pw, path))
 		return -1;
-	}
 
 	if (! bsnprintf(tmp, sizeof(tmp), "%s/tmp/%s", path,
-		batchp->message.message_uid)) {
-		batchp->message.status |= S_MESSAGE_TEMPFAILURE;
-		return -1;
-	}
-
-	fd = open(tmp, mode, 0600);
-	if (fd == -1) {
-		batchp->message.status |= S_MESSAGE_TEMPFAILURE;
+		batchp->message.message_uid))
 		return -1;
-	}
 
-	return fd;
-}
-
-int
-parent_mailfile_rename(struct batch *batchp, struct path *path)
-{
-	char srcpath[MAXPATHLEN];
-	char dstpath[MAXPATHLEN];
-
-	if (! bsnprintf(srcpath, sizeof(srcpath), "%s/tmp/%s",
-		path->rule.r_value.path, batchp->message.message_uid) ||
-	    ! bsnprintf(dstpath, sizeof(dstpath), "%s/new/%s",
-		path->rule.r_value.path, batchp->message.message_uid))
-		return 0;
-
-	if (rename(srcpath, dstpath) == -1) {
-		if (unlink(srcpath) == -1)
-			fatal("unlink");
-		batchp->message.status |= S_MESSAGE_TEMPFAILURE;
-		return 0;
-	}
-
-	return 1;
+	return open(tmp, mode, 0600);
 }
 
 int
 parent_external_mda(char *path, struct passwd *pw, struct batch *batchp)
 {
+	struct child *child;
 	pid_t pid;
 	int pipefd[2];
 	arglist args;
@@ -1321,7 +1328,6 @@ parent_external_mda(char *path, struct passwd *pw, struct batch *batchp)
 	if (pipe(pipefd) == -1) {
 		if (errno == ENFILE) {
 			log_warn("parent_external_mda: pipe");
-			batchp->message.status |= S_MESSAGE_TEMPFAILURE;
 			return -1;
 		}
 		fatal("parent_external_mda: pipe");
@@ -1332,7 +1338,6 @@ parent_external_mda(char *path, struct passwd *pw, struct batch *batchp)
 		log_warn("parent_external_mda: fork");
 		close(pipefd[0]);
 		close(pipefd[1]);
-		batchp->message.status |= S_MESSAGE_TEMPFAILURE;
 		return -1;
 	}
 
@@ -1364,6 +1369,9 @@ parent_external_mda(char *path, struct passwd *pw, struct batch *batchp)
 		if (closefrom(STDERR_FILENO + 1) == -1)
 			fatal("closefrom");
 
+		/* avoid hangs by setting a 5m timeout */
+		alarm(300);
+
 		envp[0] = "PATH=" _PATH_DEFPATH;
 		envp[1] = (char *)NULL;
 		environ = envp;
@@ -1372,7 +1380,10 @@ parent_external_mda(char *path, struct passwd *pw, struct batch *batchp)
 		_exit(1);
 	}
 
-	child_add(batchp->env, pid, CHILD_MDA, -1);
+	child = child_add(batchp->env, pid, CHILD_MDA, -1);
+
+	/* Each child relates to a batch; record this relationship. */
+	child->mda_batch = *batchp;
 
 	close(pipefd[0]);
 	return pipefd[1];
@@ -1496,26 +1507,8 @@ parent_filename_open(char *path, struct passwd *pw, struct batch *batchp)
 
 	fd = open(path, mode, 0600);
 	if (fd == -1) {
-		/* XXX - this needs to be discussed ... */
-		switch (errno) {
-		case ENOTDIR:
-		case ENOENT:
-		case EACCES:
-		case ELOOP:
-		case EROFS:
-		case EDQUOT:
-		case EINTR:
-		case EIO:
-		case EMFILE:
-		case ENFILE:
-		case ENOSPC:
-			batchp->message.status |= S_MESSAGE_TEMPFAILURE;
-			break;
-		case EWOULDBLOCK:
+		if (errno == EWOULDBLOCK)
 			goto lockfail;
-		default:
-			batchp->message.status |= S_MESSAGE_PERMFAILURE;
-		}
 		return -1;
 	}
 
@@ -1531,7 +1524,7 @@ lockfail:
 	if (fd != -1)
 		close(fd);
 
-	batchp->message.status |= S_MESSAGE_TEMPFAILURE|S_MESSAGE_LOCKFAILURE;
+	batchp->message.status |= S_MESSAGE_LOCKFAILURE;
 	return -1;
 }
 
@@ -1590,4 +1583,27 @@ child_cmp(struct child *c1, struct child *c2)
 	return (0);
 }
 
+void
+parent_mda_permfail(struct smtpd *env, struct batch *b)
+{
+	b->message.status |= S_MESSAGE_PERMFAILURE;
+	imsg_compose_event(env->sc_ievs[PROC_MDA], IMSG_MDA_FINALIZE,
+	    0, 0, -1, b, sizeof(*b));
+}
+
+void
+parent_mda_tempfail(struct smtpd *env, struct batch *b)
+{
+	imsg_compose_event(env->sc_ievs[PROC_MDA], IMSG_MDA_FINALIZE,
+	    0, 0, -1, b, sizeof(*b));
+}
+
+void
+parent_mda_success(struct smtpd *env, struct batch *b)
+{
+	b->message.status &= ~S_MESSAGE_TEMPFAILURE;
+	imsg_compose_event(env->sc_ievs[PROC_MDA], IMSG_MDA_FINALIZE,
+	    0, 0, -1, b, sizeof(*b));
+}
+
 SPLAY_GENERATE(childtree, child, entry, child_cmp);
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index 60ecd2e5ec2..d539be9b01c 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.h,v 1.138 2009/09/02 12:47:06 jacekm Exp $	*/
+/*	$OpenBSD: smtpd.h,v 1.139 2009/09/04 11:49:23 jacekm Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -145,8 +145,7 @@ enum imsg_type {
 	IMSG_LKA_MAIL,
 	IMSG_LKA_RCPT,
 	IMSG_LKA_SECRET,
-	IMSG_MDA_MAILBOX_FILE,
-	IMSG_MDA_MESSAGE_FILE,
+	IMSG_MDA_FINALIZE,
 	IMSG_MFA_RCPT,
 	IMSG_MFA_MAIL,
 
@@ -175,7 +174,8 @@ enum imsg_type {
 	IMSG_PARENT_FORWARD_OPEN,
 	IMSG_PARENT_MAILBOX_OPEN,
 	IMSG_PARENT_MESSAGE_OPEN,
-	IMSG_PARENT_MAILBOX_RENAME,
+	IMSG_PARENT_MAILDIR_RENAME,
+	IMSG_PARENT_MAILDIR_FAIL,
 	IMSG_PARENT_STATS,
 
 	IMSG_PARENT_AUTHENTICATE,
@@ -455,21 +455,6 @@ enum batch_type {
 	T_BOUNCE_BATCH		= 0x4
 };
 
-enum child_type {
-	CHILD_INVALID,
-	CHILD_DAEMON,
-	CHILD_MDA,
-	CHILD_ENQUEUE_OFFLINE
-};
-
-struct child {
-	SPLAY_ENTRY(child)	entry;
-
-	pid_t			pid;
-	enum child_type		type;
-	enum smtp_proc_type	title;
-};
-
 struct batch {
 	SPLAY_ENTRY(batch)	 b_nodes;
 
@@ -490,9 +475,29 @@ struct batch {
 	FILE			*messagefp;
 	TAILQ_HEAD(, message)	 messages;
 
+	int			 mboxfd;
+	int			 datafd;
+	int			 cleanup_parent;
+
 	enum batch_status	 status;
 };
 
+enum child_type {
+	CHILD_INVALID,
+	CHILD_DAEMON,
+	CHILD_MDA,
+	CHILD_ENQUEUE_OFFLINE
+};
+
+struct child {
+	SPLAY_ENTRY(child)	 entry;
+
+	pid_t			 pid;
+	enum child_type		 type;
+	enum smtp_proc_type	 title;
+	struct batch		 mda_batch;
+};
+
 enum session_state {
 	S_INIT = 0,
 	S_GREETED,
@@ -882,13 +887,6 @@ void		 session_bufferevent_new(struct session *);
 
 SPLAY_PROTOTYPE(sessiontree, session, s_nodes, session_cmp);
 
-/* store.c */
-int file_copy(FILE *, FILE *, struct path *, enum action_type, int);
-int store_write_header(struct batch *, struct message *, FILE *, int);
-int store_write_message(struct batch *, struct message *);
-int store_write_daemon(struct batch *, struct message *);
-int store_message(struct batch *, struct message *);
-
 /* config.c */
 #define		 PURGE_LISTENERS	0x01
 #define		 PURGE_MAPS		0x02
diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile
index 943f1dd51a3..9a4b0fe1390 100644
--- a/usr.sbin/smtpd/smtpd/Makefile
+++ b/usr.sbin/smtpd/smtpd/Makefile
@@ -1,11 +1,11 @@
-#	$OpenBSD: Makefile,v 1.12 2009/08/27 11:37:30 jacekm Exp $
+#	$OpenBSD: Makefile,v 1.13 2009/09/04 11:49:23 jacekm Exp $
 
 PROG=		smtpd
 SRCS=		aliases.c authenticate.c bounce.c buffer.c client.c	\
 		config.c control.c dns.c forward.c imsg.c lka.c log.c	\
 		map.c mda.c mfa.c mta.c parse.y queue.c queue_shared.c	\
 		ruleset.c runner.c smtp.c smtp_session.c smtpd.c ssl.c	\
-		ssl_privsep.c store.c util.c
+		ssl_privsep.c util.c
 
 MAN=		smtpd.8 smtpd.conf.5
 BINDIR=		/usr/sbin