]> git.ipfire.org Git - thirdparty/dhcpcd.git/commitdiff
dhcpcd: Redirect stdout/stderr to the launcher stderr descriptor
authorRoy Marples <roy@marples.name>
Sun, 6 Sep 2020 01:41:08 +0000 (02:41 +0100)
committerRoy Marples <roy@marples.name>
Sun, 6 Sep 2020 01:41:08 +0000 (02:41 +0100)
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.

src/dhcpcd.c
src/logerr.c
src/logerr.h
src/privsep.c

index c8589fe308028e86b20c3457243a55cbaabf805c..d5d67f64d4e229bb0df639afdaf75d15d3989195 100644 (file)
@@ -361,7 +361,7 @@ dhcpcd_daemonise(struct dhcpcd_ctx *ctx)
 
        /* 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 @@ dhcpcd_daemonise(struct dhcpcd_ctx *ctx)
        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 @@ printpidfile:
        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 @@ printpidfile:
                        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;
index 872033b62278396a5a403a8b9386cb3ef2bef223..817c53ca4d1bd76868f96e7e00e2b02594273b5f 100644 (file)
@@ -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 @@ vlogprintf_r(struct logctx *ctx, FILE *stream, const char *fmt, va_list args)
        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 @@ vlogprintf_r(struct logctx *ctx, FILE *stream, const char *fmt, va_list args)
        }
 
 #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 @@ vlogprintf_r(struct logctx *ctx, FILE *stream, const char *fmt, va_list args)
        }
 #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 @@ vlogmessage(int pri, const char *fmt, va_list args)
            (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;
@@ -369,31 +362,6 @@ logsettag(const char *tag)
 }
 #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)
 {
index 82368d1f5b473568ad0ea1f56ff96a82734540fa..4b4d6dc42566418e531b4eceff52594eed5d3e8e 100644 (file)
@@ -97,8 +97,6 @@ void logsetopts(unsigned int);
 void logsettag(const char *);
 #endif
 
-int loggeterrfd(void);
-int logseterrfd(int);
 int logopen(const char *);
 void logclose(void);
 int logreopen(void);
index 2319ac579f9835701dd7dc78b20f52f1de9ed75c..bdef041f0a8f2fff7e002bf0868014304a2563f5 100644 (file)
@@ -163,7 +163,7 @@ ps_dropprivs(struct dhcpcd_ctx *ctx)
        /* 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");
        }