# HG changeset patch # User Roy Marples # Date 1599356468 -3600 # Node ID ff7c7b4799b3ec9781fe6684c51e4b358f2ea0cf # Parent 248013138b09e08f3c3169d7292a62759c720817 dhcpcd: Redirect stdout/stderr to the launcher stderr descriptor This actually make life really simple! We no longer need to redirect stdout/stderr to /dev/null for privsep and any script output is now captured again - and it all goes to stderr as it should even if a script wants it to go to stdout. On the happy path, only the master process will actually log anything to stderr so we turn that off after we "fork". On the unhappy path, logging to stderr/stdout *may* fail because the launcher process *may* have exited. We *could* have the master process as an intermediary but that's just excess code to avoid errors which *should* not happen. Regardless, any errror should still hit syslog. diff -r 248013138b09 -r ff7c7b4799b3 src/dhcpcd.c --- a/src/dhcpcd.c Sat Sep 05 17:01:59 2020 +0100 +++ b/src/dhcpcd.c Sun Sep 06 02:41:08 2020 +0100 @@ -361,7 +361,7 @@ /* Don't use loginfo because this makes no sense in a log. */ if (!(logopts & LOGERR_QUIET)) - (void)dprintf(loggeterrfd(), + (void)fprintf(stderr, "forked to background, child pid %d\n", getpid()); i = EXIT_SUCCESS; if (write(ctx->fork_fd, &i, sizeof(i)) == -1) @@ -371,11 +371,18 @@ close(ctx->fork_fd); ctx->fork_fd = -1; - if (isatty(loggeterrfd())) { - logopts &= ~LOGERR_ERR; - logsetopts(logopts); - logseterrfd(-1); - } + /* + * Stop writing to stderr. + * On the happy path, only the master process writes to stderr, + * so this just stops wasting fprintf calls to nowhere. + * All other calls - ie errors in privsep processes or script output, + * will error when printing. + * If we *really* want to fix that, then we need to suck + * stderr/stdout in the master process and either disacrd it or pass + * it to the launcher process and then to stderr. + */ + logopts &= ~LOGERR_ERR; + logsetopts(logopts); #endif } @@ -2253,8 +2260,6 @@ case 0: ctx.fork_fd = fork_fd[1]; close(fork_fd[0]); - logseterrfd(stderr_fd[1]); - close(stderr_fd[0]); #ifdef PRIVSEP_RIGHTS if (ps_rights_limit_fd(fork_fd[1]) == -1 || ps_rights_limit_fd(stderr_fd[1]) == 1) @@ -2263,9 +2268,15 @@ goto exit_failure; } #endif - if (freopen(_PATH_DEVNULL, "w", stdout) == NULL || - freopen(_PATH_DEVNULL, "w", stderr) == NULL) - logerr("freopen"); + /* Redirect stderr to the stderr socketpair. + * Redirect stdout as well. + * dhcpcd doesn't output via stdout, but something in + * a called script might. */ + if (dup2(stderr_fd[1], STDERR_FILENO) == -1 || + dup2(stderr_fd[1], STDOUT_FILENO) == -1) + logerr("dup2"); + close(stderr_fd[0]); + close(stderr_fd[1]); if (setsid() == -1) { logerr("%s: setsid", __func__); goto exit_failure; diff -r 248013138b09 -r ff7c7b4799b3 src/logerr.c --- a/src/logerr.c Sat Sep 05 17:01:59 2020 +0100 +++ b/src/logerr.c Sun Sep 06 02:41:08 2020 +0100 @@ -52,7 +52,6 @@ struct logctx { char log_buf[BUFSIZ]; unsigned int log_opts; - FILE *log_err; #ifndef SMALL FILE *log_file; #ifdef LOGERR_TAG @@ -120,14 +119,13 @@ int len = 0, e; va_list a; #ifndef SMALL - FILE *err = ctx->log_err == NULL ? stderr : ctx->log_err; bool log_pid; #ifdef LOGERR_TAG bool log_tag; #endif - if ((stream == err && ctx->log_opts & LOGERR_ERR_DATE) || - (stream != err && ctx->log_opts & LOGERR_LOG_DATE)) + if ((stream == stderr && ctx->log_opts & LOGERR_ERR_DATE) || + (stream != stderr && ctx->log_opts & LOGERR_LOG_DATE)) { if ((e = logprintdate(stream)) == -1) return -1; @@ -135,8 +133,8 @@ } #ifdef LOGERR_TAG - log_tag = ((stream == err && ctx->log_opts & LOGERR_ERR_TAG) || - (stream != err && ctx->log_opts & LOGERR_LOG_TAG)); + log_tag = ((stream == stderr && ctx->log_opts & LOGERR_ERR_TAG) || + (stream != stderr && ctx->log_opts & LOGERR_LOG_TAG)); if (log_tag) { if (ctx->log_tag == NULL) ctx->log_tag = getprogname(); @@ -146,8 +144,8 @@ } #endif - log_pid = ((stream == err && ctx->log_opts & LOGERR_ERR_PID) || - (stream != err && ctx->log_opts & LOGERR_LOG_PID)); + log_pid = ((stream == stderr && ctx->log_opts & LOGERR_ERR_PID) || + (stream != stderr && ctx->log_opts & LOGERR_LOG_PID)); if (log_pid) { if ((e = fprintf(stream, "[%d]", getpid())) == -1) return -1; @@ -204,12 +202,7 @@ (pri <= LOG_ERR || (!(ctx->log_opts & LOGERR_QUIET) && pri <= LOG_INFO) || (ctx->log_opts & LOGERR_DEBUG && pri <= LOG_DEBUG))) - { - FILE *err; - - err = ctx->log_err == NULL ? stderr : ctx->log_err; - len = vlogprintf_r(ctx, err, fmt, args); - } + len = vlogprintf_r(ctx, stderr, fmt, args); if (!(ctx->log_opts & LOGERR_LOG)) return len; @@ -370,31 +363,6 @@ #endif int -loggeterrfd(void) -{ - struct logctx *ctx = &_logctx; - FILE *err = ctx->log_err == NULL ? stderr : ctx->log_err; - - return fileno(err); -} - -int -logseterrfd(int fd) -{ - struct logctx *ctx = &_logctx; - - if (ctx->log_err != NULL) - fclose(ctx->log_err); - if (fd == -1) { - ctx->log_err = NULL; - return 0; - } - ctx->log_err = fdopen(fd, "a"); - setlinebuf(ctx->log_err); - return ctx->log_err == NULL ? -1 : 0; -} - -int logopen(const char *path) { struct logctx *ctx = &_logctx; diff -r 248013138b09 -r ff7c7b4799b3 src/logerr.h --- a/src/logerr.h Sat Sep 05 17:01:59 2020 +0100 +++ b/src/logerr.h Sun Sep 06 02:41:08 2020 +0100 @@ -97,8 +97,6 @@ void logsettag(const char *); #endif -int loggeterrfd(void); -int logseterrfd(int); int logopen(const char *); void logclose(void); int logreopen(void); diff -r 248013138b09 -r ff7c7b4799b3 src/privsep.c --- a/src/privsep.c Sat Sep 05 17:01:59 2020 +0100 +++ b/src/privsep.c Sun Sep 06 02:41:08 2020 +0100 @@ -163,7 +163,7 @@ /* Prohibit writing to files. * Obviously this won't work if we are using a logfile * or redirecting stderr to a file. */ - if (ctx->logfile == NULL && isatty(loggeterrfd())) { + if (ctx->logfile == NULL) { if (setrlimit(RLIMIT_FSIZE, &rzero) == -1) logerr("setrlimit RLIMIT_FSIZE"); }