From: Roy Marples <roy@marples.name>
Date: Sun, 6 Sep 2020 01:41:08 +0000 (+0100)
Subject: dhcpcd: Redirect stdout/stderr to the launcher stderr descriptor
X-Git-Tag: v9.2.0~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=71b878695ef0557a036fee35997e52cd1f3d2aea;p=thirdparty%2Fdhcpcd.git

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 --git a/src/dhcpcd.c b/src/dhcpcd.c
index c8589fe3..d5d67f64 100644
--- a/src/dhcpcd.c
+++ b/src/dhcpcd.c
@@ -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;
diff --git a/src/logerr.c b/src/logerr.c
index 872033b6..817c53ca 100644
--- a/src/logerr.c
+++ b/src/logerr.c
@@ -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)
 {
diff --git a/src/logerr.h b/src/logerr.h
index 82368d1f..4b4d6dc4 100644
--- a/src/logerr.h
+++ b/src/logerr.h
@@ -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);
diff --git a/src/privsep.c b/src/privsep.c
index 2319ac57..bdef041f 100644
--- a/src/privsep.c
+++ b/src/privsep.c
@@ -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");
 	}