From: Christian Kastner 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 . 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);