Mercurial > hg > dhcpcd
changeset 5441:ff7c7b4799b3 draft
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.
| author | Roy Marples <roy@marples.name> |
|---|---|
| date | Sun, 06 Sep 2020 02:41:08 +0100 |
| parents | 248013138b09 |
| children | a069d919d44c |
| files | src/dhcpcd.c src/logerr.c src/logerr.h src/privsep.c |
| diffstat | 4 files changed, 30 insertions(+), 53 deletions(-) [+] |
line wrap: on
line diff
--- 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;
--- 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;
--- 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);
--- 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"); }