Replace popen/setjmp/pclose with a manual pipe/fork/exec/wait.

We can limit the time we wait on wall(1) without the complexity
inherent to setjmp.

Actually wait (instead of waitpid) to pick up any straggler wall
processes from prior timewarn() calls.

With a tweak from millert@ to ensure we don't accidentally close
stdin before we exec wall.

ok millert@ tb@

1 files changed, 50 insertions, 28 deletions

diff --git a/sbin/shutdown/shutdown.c b/sbin/shutdown/shutdown.c
index 7f957f6ab2d..1689bb32430 100644
--- a/sbin/shutdown/shutdown.c
+++ b/sbin/shutdown/shutdown.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: shutdown.c,v 1.47 2018/02/04 04:28:41 cheloha Exp $	*/
+/*	$OpenBSD: shutdown.c,v 1.48 2018/02/24 20:00:07 cheloha Exp $	*/
 /*	$NetBSD: shutdown.c,v 1.9 1995/03/18 15:01:09 cgd Exp $	*/
 
 /*
@@ -40,7 +40,6 @@
 #include <fcntl.h>
 #include <sys/termios.h>
 #include <pwd.h>
-#include <setjmp.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -86,7 +85,7 @@ struct interval {
 
 static time_t offset, shuttime;
 static int dofast, dohalt, doreboot, dopower, dodump, mbuflen, nosync;
-static sig_atomic_t killflg;
+static sig_atomic_t killflg, timed_out;
 static char *whom, mbuf[BUFSIZ];
 
 void badtime(void);
@@ -268,8 +267,6 @@ loop(void)
 	die_you_gravy_sucking_pig_dog();
 }
 
-static jmp_buf alarmbuf;
-
 static char *restricted_environ[] = {
 	"PATH=" _PATH_STDPATH,
 	NULL
@@ -279,59 +276,84 @@ void
 timewarn(int timeleft)
 {
 	static char hostname[HOST_NAME_MAX+1];
-	char wcmd[PATH_MAX + 4];
-	extern char **environ;
 	static int first;
-	FILE *pf;
+	int fd[2];
+	pid_t pid, wpid;
 
 	if (!first++)
 		(void)gethostname(hostname, sizeof(hostname));
 
-	/* undoc -n option to wall suppresses normal wall banner */
-	(void)snprintf(wcmd, sizeof(wcmd), "%s -n", _PATH_WALL);
-	environ = restricted_environ;
-	if (!(pf = popen(wcmd, "w"))) {
-		syslog(LOG_ERR, "shutdown: can't find %s: %m", _PATH_WALL);
+	if (pipe(fd) == -1) {
+		syslog(LOG_ERR, "pipe: %m");
+		return;
+	}
+	switch (pid = fork()) {
+	case -1:
+		syslog(LOG_ERR, "fork: %m");
+		close(fd[0]);
+		close(fd[1]);
 		return;
+	case 0:
+		if (dup2(fd[0], STDIN_FILENO) == -1) {
+			syslog(LOG_ERR, "dup2: %m");
+			_exit(1);
+		}
+		if (fd[0] != STDIN_FILENO)
+			close(fd[0]);
+		close(fd[1]);
+		/* wall(1)'s undocumented '-n' flag suppresses its banner. */
+		execle(_PATH_WALL, _PATH_WALL, "-n", (char *)NULL,
+		    restricted_environ);
+		syslog(LOG_ERR, "%s: %m", _PATH_WALL);
+		_exit(1);
+	default:
+		close(fd[0]);
 	}
 
-	(void)fprintf(pf,
+	dprintf(fd[1],
 	    "\007*** %sSystem shutdown message from %s@%s ***\007\n",
 	    timeleft ? "": "FINAL ", whom, hostname);
 
 	if (timeleft > 10*60) {
 		struct tm *tm = localtime(&shuttime);
 
-		fprintf(pf, "System going down at %d:%02d\n\n",
+		dprintf(fd[1], "System going down at %d:%02d\n\n",
 		    tm->tm_hour, tm->tm_min);
 	} else if (timeleft > 59)
-		(void)fprintf(pf, "System going down in %d minute%s\n\n",
+		dprintf(fd[1], "System going down in %d minute%s\n\n",
 		    timeleft / 60, (timeleft > 60) ? "s" : "");
 	else if (timeleft)
-		(void)fprintf(pf, "System going down in 30 seconds\n\n");
+		dprintf(fd[1], "System going down in 30 seconds\n\n");
 	else
-		(void)fprintf(pf, "System going down IMMEDIATELY\n\n");
+		dprintf(fd[1], "System going down IMMEDIATELY\n\n");
 
 	if (mbuflen)
-		(void)fwrite(mbuf, sizeof(*mbuf), mbuflen, pf);
+		(void)write(fd[1], mbuf, mbuflen);
+	close(fd[1]);
 
 	/*
-	 * play some games, just in case wall doesn't come back
-	 * probably unnecessary, given that wall is careful.
+	 * If we wait longer than 30 seconds for wall(1) to exit we'll
+	 * throw off our schedule.
 	 */
-	if (!setjmp(alarmbuf)) {
-		(void)signal(SIGALRM, timeout);
-		(void)alarm((u_int)30);
-		(void)pclose(pf);
-		(void)alarm((u_int)0);
-		(void)signal(SIGALRM, SIG_DFL);
+	signal(SIGALRM, timeout);
+	siginterrupt(SIGALRM, 1);
+	alarm(30);
+	while ((wpid = wait(NULL)) != pid && !timed_out)
+		continue;
+	alarm(0);
+	signal(SIGALRM, SIG_DFL);
+	if (timed_out) {
+		syslog(LOG_NOTICE,
+		    "wall[%ld] is taking too long to exit; continuing",
+		    (long)pid);
+		timed_out = 0;
 	}
 }
 
 void
 timeout(int signo)
 {
-	longjmp(alarmbuf, 1);		/* XXX signal/longjmp resource leaks */
+	timed_out = 1;
 }
 
 void