From: Christian Kastner <ckk@kvr.at>
Date: Fri, 15 Jan 2016 22:06:27 +0100
Subject: Avoid a mailcmd timeout

Long running cron jobs can cause sendmail (or whatever compatible mailer is
installed) to time out. By writing children's output to a temporary file 
and waiting for them to terminate instead, this situation can be avoided.

Contributed by Justin Pryzby <justinpryzby@users.sourceforge.net>.

Bug-Debian: https://bugs.debian.org/155109
Forwarded: no
Last-Update: 2016-01-15
---
 do_command.c | 342 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 168 insertions(+), 174 deletions(-)

diff --git a/do_command.c b/do_command.c
index f646722..cbca2d1 100644
--- a/do_command.c
+++ b/do_command.c
@@ -111,12 +111,21 @@ do_command(e, u)
 }
 
 
+/*
+ * CROND
+ *  - cron (runs child_process);
+ *    - cron (runs exec sh -c 'tab entry');
+ *    - cron (writes any %-style stdin to the command);
+ *    - mail (popen writes any stdout to mailcmd);
+ */
+
 static void
 child_process(e, u)
 	entry	*e;
 	user	*u;
 {
-	int		stdin_pipe[2], stdout_pipe[2];
+	int		stdin_pipe[2];
+	FILE		*tmpout;
 	register char	*input_data;
 	char		*usernm, *mailto;
 	int		children = 0;
@@ -156,10 +165,14 @@ child_process(e, u)
 	(void) signal(SIGCLD, SIG_DFL);
 #endif /*BSD*/
 
-	/* create some pipes to talk to our future child
+	/* create a pipe to talk to our future child
 	 */
 	pipe(stdin_pipe);	/* child's stdin */
-	pipe(stdout_pipe);	/* child's stdout */
+	/* child's stdout */
+	if ((tmpout = tmpfile()) == NULL) {
+		log_it("CRON", getpid(), "error", "create tmpfile");
+		exit(ERROR_EXIT);
+	}
 	
 	/* since we are a forked process, we can diddle the command string
 	 * we were passed -- nobody else is going to use it again, right?
@@ -249,20 +262,19 @@ child_process(e, u)
 		 * appropriate circumstances.
 		 */
 		close(stdin_pipe[WRITE_PIPE]);
-		close(stdout_pipe[READ_PIPE]);
 
 		/* grandchild process.  make std{in,out} be the ends of
 		 * pipes opened by our daddy; make stderr go to stdout.
 		 */
 		dup2(stdin_pipe[READ_PIPE], STDIN);
-		dup2(stdout_pipe[WRITE_PIPE], STDOUT);
+		dup2(fileno(tmpout), STDOUT);
 		dup2(STDOUT, STDERR);
 
 
-		/* close the pipes we just dup'ed.  The resources will remain.
+		/* close the pipe we just dup'ed.  The resources will remain.
 		 */
 		close(stdin_pipe[READ_PIPE]);
-		close(stdout_pipe[WRITE_PIPE]);
+		fclose(tmpout);
 
 		/* set our login universe.  Do this in the grandchild
 		 * so that the child can invoke /usr/lib/sendmail
@@ -354,11 +366,10 @@ child_process(e, u)
 
 	Debug(DPROC, ("[%d] child continues, closing pipes\n", getpid()))
 
-	/* close the ends of the pipe that will only be referenced in the
+	/* close the end of the pipe that will only be referenced in the
 	 * grandchild process...
 	 */
 	close(stdin_pipe[READ_PIPE]);
-	close(stdout_pipe[WRITE_PIPE]);
 
 	/*
 	 * write, to the pipe connected to child's stdin, any input specified
@@ -379,11 +390,6 @@ child_process(e, u)
 
 		Debug(DPROC, ("[%d] child2 sending data to grandchild\n", getpid()))
 
-		/* close the pipe we don't use, since we inherited it and
-		 * are part of its reference count now.
-		 */
-		close(stdout_pipe[READ_PIPE]);
-
 		/* translation:
 		 *	\% -> %
 		 *	%  -> \n
@@ -431,183 +437,171 @@ child_process(e, u)
 	 * when the grandchild exits, we'll get EOF.
 	 */
 
-	Debug(DPROC, ("[%d] child reading output from grandchild\n", getpid()))
+	/* wait for children to die.
+	 */
+	int status = 0;
+	for (; children > 0; children--)
+	{
+		char		msg[256];
+		WAIT_T		waiter;
+		PID_T		pid;
 
-	/*local*/{
-		register FILE	*in = fdopen(stdout_pipe[READ_PIPE], "r");
-		register int	ch = getc(in);
-
-		if (ch != EOF) {
-			register FILE	*mail = NULL;
-			register int	bytes = 1;
-			int		status = 0;
-
-			Debug(DPROC|DEXT,
-				("[%d] got data (%x:%c) from grandchild\n",
-					getpid(), ch, ch))
-
-			/* get name of recipient.  this is MAILTO if set to a
-			 * valid local username; USER otherwise.
-			 */
-			if (mailto) {
-				/* MAILTO was present in the environment
-				 */
-				if (!*mailto) {
-					/* ... but it's empty. set to NULL
-					 */
-					mailto = NULL;
-				}
-			} else {
-				/* MAILTO not present, set to USER.
-				 */
-				mailto = usernm;
-			}
-		
-			/* if we are supposed to be mailing, MAILTO will
-			 * be non-NULL.  only in this case should we set
-			 * up the mail command and subjects and stuff...
-			 */
-
-			if (mailto) {
-				register char	**env;
-				char		**jobenv = build_env(e->envp);
-				auto char	mailcmd[MAX_COMMAND];
-				auto char	hostname[MAXHOSTNAMELEN];
-				char	*content_type = env_get("CONTENT_TYPE",jobenv),
-					*content_transfer_encoding = env_get("CONTENT_TRANSFER_ENCODING",jobenv);
-
-
-				(void) gethostname(hostname, MAXHOSTNAMELEN);
-				(void) snprintf(mailcmd, sizeof(mailcmd),
-						MAILARGS, MAILCMD, mailto);
-				if (!(mail = cron_popen(mailcmd, "w", e))) {
-					perror(MAILCMD);
-					(void) _exit(ERROR_EXIT);
-				}
-				fprintf(mail, "From: root (Cron Daemon)\n");
-				fprintf(mail, "To: %s\n", mailto);
-				fprintf(mail, "Subject: Cron <%s@%s> %s\n",
-					usernm, first_word(hostname, "."),
-					e->cmd);
+		Debug(DPROC, ("[%d] waiting for grandchild #%d to finish\n",
+			getpid(), children))
+		pid = wait(&waiter);
+		if (pid < OK) {
+			Debug(DPROC, ("[%d] no more grandchildren\n", getpid()))
+			break;
+		}
+		Debug(DPROC, ("[%d] grandchild #%d finished, status=%04x\n",
+			getpid(), pid, WEXITSTATUS(waiter)))
+
+		if (WIFEXITED(waiter) && WEXITSTATUS(waiter)) {
+			status = waiter;
+			snprintf(msg, 256, "grandchild #%d failed with exit "
+				"status %d", pid, WEXITSTATUS(waiter));
+			log_it("CRON", getpid(), "error", msg);
+		} else if (WIFSIGNALED(waiter)) {
+			status = waiter;
+			snprintf(msg, 256, "grandchild #%d terminated by signal"
+				" %d%s", pid, WTERMSIG(waiter),
+				WCOREDUMP(waiter) ? ", dumped core" : "");
+			log_it("CRON", getpid(), "error", msg);
+		}
+	}
+
+// Finally, send any output of the command to the mailer; also, alert
+// the user if their job failed.  Avoid popening the mailcmd until now
+// since sendmail may time out, and to write info about the exit
+// status.
+
+	long pos;
+
+	fseek(tmpout, 0, SEEK_END);
+	pos = ftell(tmpout);
+	fseek(tmpout, 0, SEEK_SET);
+
+	Debug(DPROC|DEXT, ("[%d] got %ld bytes data from grandchild tmpfile\n",
+				getpid(), (long) pos))
+	if (pos == 0)
+		goto mail_finished;
+
+	// get name of recipient.
+	if (mailto == NULL)
+		mailto = usernm;
+	else if (!*mailto)
+                goto mail_finished;
+
+	register FILE	*mail = NULL;
+	register int	bytes = 0;
+
+	register char	**env;
+	char		**jobenv = build_env(e->envp);
+	auto char	mailcmd[MAX_COMMAND];
+	auto char	hostname[MAXHOSTNAMELEN];
+	char		*content_type = env_get("CONTENT_TYPE",jobenv),
+			*content_transfer_encoding = env_get("CONTENT_TRANSFER_ENCODING",jobenv);
+
+	(void) gethostname(hostname, MAXHOSTNAMELEN);
+	(void) snprintf(mailcmd, sizeof(mailcmd),
+			MAILARGS, MAILCMD, mailto);
+	if (!(mail = cron_popen(mailcmd, "w", e))) {
+		perror(MAILCMD);
+		(void) _exit(ERROR_EXIT);
+	}
+	fprintf(mail, "From: root (Cron Daemon)\n");
+	fprintf(mail, "To: %s\n", mailto);
+	fprintf(mail, "Subject: Cron <%s@%s> %s%s\n",
+			usernm, first_word(hostname, "."),
+			e->cmd, status?" (failed)":"");
 # if defined(MAIL_DATE)
-				fprintf(mail, "Date: %s\n",
-					arpadate(&StartTime));
+	fprintf(mail, "Date: %s\n",
+			arpadate(&StartTime));
 # endif /* MAIL_DATE */
-				fprintf(mail, "MIME-Version: 1.0\n");
-
-				if (content_type == 0L) {
-					fprintf(mail, "Content-Type: text/plain; charset=%s\n",
-						cron_default_mail_charset);
-				} else {
-					/* user specified Content-Type header.
-					 * disallow new-lines for security reasons
-					 * (else users could specify arbitrary mail headers!)
-					 */
-					char *nl=content_type;
-					size_t ctlen = strlen(content_type);
-
-					while ((*nl != '\0') &&
-							((nl=strchr(nl,'\n')) != 0L) &&
-							(nl < (content_type+ctlen))) {
-						*nl = ' ';
-					}
-				       fprintf(mail,"Content-Type: %s\n", content_type);
-				}
-
-				if (content_transfer_encoding != 0L) {
-					char *nl=content_transfer_encoding;
-					size_t ctlen = strlen(content_transfer_encoding);
-					while ((*nl != '\0') &&
-							((nl=strchr(nl,'\n')) != 0L) &&
-							(nl < (content_transfer_encoding+ctlen))) {
-						*nl = ' ';
-					}
+	fprintf(mail, "MIME-Version: 1.0\n");
+
+	if (content_type == 0L) {
+		fprintf(mail, "Content-Type: text/plain; charset=%s\n",
+			cron_default_mail_charset);
+	} else {
+		/* user specified Content-Type header.
+		 * disallow new-lines for security reasons
+		 * (else users could specify arbitrary mail headers!)
+		 */
+		char *nl = content_type;
+		size_t ctlen = strlen(content_type);
 
-					fprintf(mail,"Content-Transfer-Encoding: %s\n",
-							content_transfer_encoding);
-				} else {
-					fprintf(mail,"Content-Transfer-Encoding: 8bit\n");
-				}
+		while ((*nl != '\0') &&
+				((nl=strchr(nl,'\n')) != 0L) &&
+				(nl < (content_type+ctlen))) {
+			*nl = ' ';
+		}
+	       fprintf(mail,"Content-Type: %s\n", content_type);
+	}
 
-				for (env = e->envp;  *env;  env++)
-					fprintf(mail, "X-Cron-Env: <%s>\n",
-						*env);
-				fprintf(mail, "\n");
+	if (content_transfer_encoding != 0L) {
+		char *nl = content_transfer_encoding;
+		size_t ctlen = strlen(content_transfer_encoding);
+		while ((*nl != '\0') &&
+				((nl = strchr(nl,'\n')) != 0L) &&
+				(nl < (content_transfer_encoding+ctlen))) {
+			*nl = ' ';
+		}
 
-				/* this was the first char from the pipe
-				 */
-				putc(ch, mail);
-			}
+		fprintf(mail,"Content-Transfer-Encoding: %s\n",
+				content_transfer_encoding);
+	} else {
+		fprintf(mail,"Content-Transfer-Encoding: 8bit\n");
+	}
 
-			/* we have to read the input pipe no matter whether
-			 * we mail or not, but obviously we only write to
-			 * mail pipe if we ARE mailing.
-			 */
+	for (env = e->envp;  *env;  env++)
+		fprintf(mail, "X-Cron-Env: <%s>\n",
+				*env);
+	fputc('\n', mail);
 
-			while (EOF != (ch = getc(in))) {
-				bytes++;
-				if (mailto)
-					putc(ch, mail);
-			}
+// Append the actual output of the child to the mail
 
-			/* only close pipe if we opened it -- i.e., we're
-			 * mailing...
-			 */
-
-			if (mailto) {
-				Debug(DPROC, ("[%d] closing pipe to mail\n",
-					getpid()))
-				/* Note: the pclose will probably see
-				 * the termination of the grandchild
-				 * in addition to the mail process, since
-				 * it (the grandchild) is likely to exit
-				 * after closing its stdout.
-				 */
-				status = cron_pclose(mail);
-			}
+	char buf[4096];
+	int ret, remain;
 
-			/* if there was output and we could not mail it,
-			 * log the facts so the poor user can figure out
-			 * what's going on.
-			 */
-			if (mailto && status) {
-				char buf[MAX_TEMPSTR];
-
-				snprintf(buf, MAX_TEMPSTR,
-			"mailed %d byte%s of output but got status 0x%04x\n",
-					bytes, (bytes==1)?"":"s",
-					status);
-				log_it(usernm, getpid(), "MAIL", buf);
+	while(1) {
+		if ((ret = fread(buf, 1, sizeof(buf), tmpout)) == 0)
+			break;
+		for (remain = ret; remain != 0; ) {
+			ret = fwrite(buf, 1, remain, mail);
+			if (ret > 0) {
+				remain -= ret;
+				bytes += ret;
+				continue;
 			}
+			// XXX error
+			break;
+		}
+	}
 
-		} /*if data from grandchild*/
+	Debug(DPROC, ("[%d] closing pipe to mail\n", getpid()))
+	status = cron_pclose(mail);
 
-		Debug(DPROC, ("[%d] got EOF from grandchild\n", getpid()))
+	/* if there was output and we could not mail it,
+	 * log the facts so the poor user can figure out
+	 * what's going on.
+	 */
+	if (status) {
+		char buf[MAX_TEMPSTR];
+		snprintf(buf, MAX_TEMPSTR,
+				"mailed %d byte%s of output "
+				"but got status 0x%04x from MTA\n",
+				bytes, (bytes==1)?"":"s", status);
+		log_it(usernm, getpid(), "MAIL", buf);
+	}
 
-		fclose(in);	/* also closes stdout_pipe[READ_PIPE] */
+	if (ferror(tmpout)) {
+		log_it(usernm, getpid(), "MAIL", "stream error reading output");
 	}
 
-	/* wait for children to die.
-	 */
-	for (;  children > 0;  children--)
-	{
-		WAIT_T		waiter;
-		PID_T		pid;
+mail_finished:
+	fclose(tmpout);
 
-		Debug(DPROC, ("[%d] waiting for grandchild #%d to finish\n",
-			getpid(), children))
-		pid = wait(&waiter);
-		if (pid < OK) {
-			Debug(DPROC, ("[%d] no more grandchildren--mail written?\n",
-				getpid()))
-			break;
-		}
-		Debug(DPROC, ("[%d] grandchild #%d finished, status=%04x",
-			getpid(), pid, WEXITSTATUS(waiter)))
-		if (WIFSIGNALED(waiter) && WCOREDUMP(waiter))
-			Debug(DPROC, (", dumped core"))
-		Debug(DPROC, ("\n"))
-	}
 #if defined(USE_PAM)
 	pam_setcred(pamh, PAM_DELETE_CRED | PAM_SILENT);
 	retcode = pam_close_session(pamh, PAM_SILENT);