OS-6977 zoneadmd incorrectly using zerror() in child

Reviewed by: Mike Gerdts <mike.gerdts@joyent.com>
Reviewed by: Hans Rosenfeld <hans.rosenfeld@joyent.com>
Approved by: Mike Gerdts <mike.gerdts@joyent.com>

3 files changed, 30 insertions, 16 deletions

diff --git a/usr/src/cmd/zoneadmd/log.c b/usr/src/cmd/zoneadmd/log.c
index 754df3c043..ca531b7177 100644
--- a/usr/src/cmd/zoneadmd/log.c
+++ b/usr/src/cmd/zoneadmd/log.c
@@ -154,6 +154,8 @@ typedef struct jsonpair {
 	const char *jp_val;
 } jsonpair_t;
 
+boolean_t logging_poisoned = B_FALSE;
+
 /*
  * MAX_LOG_STREAMS is a small number so we allocate in the simplest way.
  */
@@ -161,7 +163,6 @@ static logstream_t streams[MAX_LOG_STREAMS];
 static logfile_t logfiles[MAX_LOG_STREAMS];
 
 static boolean_t logging_initialized = B_FALSE;
-static boolean_t logging_poisoned = B_FALSE;
 static uint64_t logging_rot_size;		/* See ZLOG_MAXSZ */
 static uint64_t logging_rot_keep;		/* See ZLOG_KEEP */
 static int logging_pending_sig = 0;		/* Signal recvd while logging */
@@ -396,15 +397,17 @@ logstream_atfork_parent(void)
 	logstream_unlock();
 }
 
+/*
+ * logstream_*() should never be called in a child process, so we make sure this
+ * code is never called there.
+ *
+ * zerror() in a child process is still safe: it knows to check for poisoning,
+ * and in such a case will redirect its output to stderr on the presumption it
+ * is a pipe to the parent.
+ */
 static void
 logstream_atfork_child(void)
 {
-	/*
-	 * This does just enough to cause any errant logging call in the child
-	 * to lead to a failed assertion.  logstream_init() must not be called
-	 * in the child.  It is expected that the child will be calling exec()
-	 * real soon.
-	 */
 	logging_poisoned = B_TRUE;
 	logging_pending_sig = 0;
 	logstream_unlock();
diff --git a/usr/src/cmd/zoneadmd/zoneadmd.c b/usr/src/cmd/zoneadmd/zoneadmd.c
index 51954a9f3e..71ded8ba0c 100644
--- a/usr/src/cmd/zoneadmd/zoneadmd.c
+++ b/usr/src/cmd/zoneadmd/zoneadmd.c
@@ -255,10 +255,20 @@ zerror(zlog_t *zlogp, boolean_t use_strerror, const char *fmt, ...)
 
 	(void) strlcat(buf, "\n", sizeof (buf));
 
-	logstream_write(platloghdl, bp_nozone, strlen(bp_nozone));
+	/*
+	 * If we don't have the platform log, we are in a child process, and
+	 * should log to stderr (which is a pipe) instead of the file.
+	 */
+	if (logging_poisoned) {
+		(void) fprintf(stderr, "%s", buf);
 
-	if (zlogp == NULL || zlogp == &logplat) {
-		return;
+		if (zlogp != &logsys && zlogp->logfile == stderr)
+			return;
+	} else {
+		logstream_write(platloghdl, bp_nozone, strlen(bp_nozone));
+
+		if (zlogp == &logplat)
+			return;
 	}
 
 	if (zlogp == &logsys) {
@@ -1094,9 +1104,7 @@ do_subproc(zlog_t *zlogp, char *cmdbuf, char **retstr, boolean_t debug)
 		sigset(SIGINT, SIG_DFL);
 
 		/*
-		 * Do not call zerror() in child process as neither zerror() nor
-		 * logstream_*() functions are designed to handle multiple
-		 * processes logging.  Rather, write all errors to the pipe.
+		 * Set up a pipe for the child to log to.
 		 */
 		if (dup2(fds[1], STDERR_FILENO) == -1) {
 			(void) snprintf(buf, sizeof (buf),
@@ -1116,19 +1124,20 @@ do_subproc(zlog_t *zlogp, char *cmdbuf, char **retstr, boolean_t debug)
 		 */
 		if ((in = open("/dev/null", O_RDONLY)) == -1 ||
 		    dup2(in, STDIN_FILENO) == -1) {
-			perror("subprocess failed to set up STDIN_FILENO");
+			zerror(zlogp, B_TRUE,
+			    "subprocess failed to set up STDIN_FILENO");
 			_exit(127);
 		}
 		closefrom(STDERR_FILENO + 1);
 
 		if (setup_subproc_env(zlogp, debug) != Z_OK) {
-			(void) fprintf(stderr, "failed to setup environment");
+			zerror(zlogp, B_FALSE, "failed to setup environment");
 			_exit(127);
 		}
 
 		(void) execl("/bin/sh", "sh", "-c", cmdbuf, NULL);
 
-		perror("subprocess execl failed");
+		zerror(zlogp, B_TRUE, "subprocess execl failed");
 		_exit(127);
 	} else if (child == -1) {
 		zerror(zlogp, B_TRUE, "failed to create subprocess for '%s'",
diff --git a/usr/src/cmd/zoneadmd/zoneadmd.h b/usr/src/cmd/zoneadmd/zoneadmd.h
index 4953479e64..471aa40a0e 100644
--- a/usr/src/cmd/zoneadmd/zoneadmd.h
+++ b/usr/src/cmd/zoneadmd/zoneadmd.h
@@ -170,6 +170,8 @@ typedef enum {
 	LS_LINE_BUFFERED = 0x1		/* Write when \n found or full buffer */
 } logstream_flags_t;
 
+extern boolean_t logging_poisoned;
+
 extern void create_log_thread(zlog_t *);
 extern void destroy_log_thread(zlog_t *);
 extern void logstream_init(zlog_t *);