upstream: There are lots of place where we want to redirect stdin,

stdout and/or stderr to /dev/null. Factor all these out to a single
stdfd_devnull() function that allows selection of which of these to redirect.
ok markus@

OpenBSD-Commit-ID: 3033ba5a4c47cacfd5def020d42cabc52fad3099

misc.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.c,v 1.153 2020/06/26 05:16:38 djm Exp $ */
+/* $OpenBSD: misc.c,v 1.154 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2005-2020 Damien Miller.  All rights reserved.
@@ -2415,3 +2415,24 @@ ssh_signal(int signum, sshsig_t handler)
 	}
 	return osa.sa_handler;
 }
+
+int
+stdfd_devnull(int do_stdin, int do_stdout, int do_stderr)
+{
+	int devnull, ret = 0;
+
+	if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
+		error("%s: open %s: %s", __func__, _PATH_DEVNULL,
+		    strerror(errno));
+		return -1;
+	}
+	if ((do_stdin && dup2(devnull, STDIN_FILENO) == -1) ||
+	    (do_stdout && dup2(devnull, STDOUT_FILENO) == -1) ||
+	    (do_stderr && dup2(devnull, STDERR_FILENO) == -1)) {
+		error("%s: dup2: %s", __func__, strerror(errno));
+		ret = -1;
+	}
+	if (devnull > STDERR_FILENO)
+		close(devnull);
+	return ret;
+}

misc.h

@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.h,v 1.87 2020/05/29 11:17:56 dtucker Exp $ */
+/* $OpenBSD: misc.h,v 1.88 2020/10/03 09:22:26 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -90,6 +90,7 @@ const char *atoi_err(const char *, int *);
 int	 parse_absolute_time(const char *, uint64_t *);
 void	 format_absolute_time(uint64_t, char *, size_t);
 int	 path_absolute(const char *);
+int	 stdfd_devnull(int, int, int);
 
 void	 sock_set_v6only(int);
 

mux.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: mux.c,v 1.83 2020/07/05 23:59:45 djm Exp $ */
+/* $OpenBSD: mux.c,v 1.84 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Copyright (c) 2002-2008 Damien Miller <djm@openbsd.org>
  *
@@ -1902,7 +1902,7 @@ mux_client_request_session(int fd)
 	const char *term;
 	u_int echar, rid, sid, esid, exitval, type, exitval_seen;
 	extern char **environ;
-	int r, i, devnull, rawmode;
+	int r, i, rawmode;
 
 	debug3("%s: entering", __func__);
 
@@ -1913,14 +1913,8 @@ mux_client_request_session(int fd)
 
 	ssh_signal(SIGPIPE, SIG_IGN);
 
-	if (stdin_null_flag) {
+	if (stdin_null_flag && stdfd_devnull(1, 0, 0) == -1)
-		if ((devnull = open(_PATH_DEVNULL, O_RDONLY)) == -1)
+		fatal("%s: stdfd_devnull failed", __func__);
-			fatal("open(/dev/null): %s", strerror(errno));
-		if (dup2(devnull, STDIN_FILENO) == -1)
-			fatal("dup2: %s", strerror(errno));
-		if (devnull > STDERR_FILENO)
-			close(devnull);
-	}
 
 	if ((term = getenv("TERM")) == NULL)
 		term = "";
@@ -2137,7 +2131,7 @@ mux_client_request_stdio_fwd(int fd)
 	struct sshbuf *m;
 	char *e;
 	u_int type, rid, sid;
-	int r, devnull;
+	int r;
 
 	debug3("%s: entering", __func__);
 
@@ -2148,14 +2142,8 @@ mux_client_request_stdio_fwd(int fd)
 
 	ssh_signal(SIGPIPE, SIG_IGN);
 
-	if (stdin_null_flag) {
+	if (stdin_null_flag && stdfd_devnull(1, 0, 0) == -1)
-		if ((devnull = open(_PATH_DEVNULL, O_RDONLY)) == -1)
+		fatal("%s: stdfd_devnull failed", __func__);
-			fatal("open(/dev/null): %s", strerror(errno));
-		if (dup2(devnull, STDIN_FILENO) == -1)
-			fatal("dup2: %s", strerror(errno));
-		if (devnull > STDERR_FILENO)
-			close(devnull);
-	}
 
 	if ((m = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new", __func__);

readconf.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.c,v 1.336 2020/10/03 08:30:47 djm Exp $ */
+/* $OpenBSD: readconf.c,v 1.337 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -509,7 +509,7 @@ execute_in_shell(const char *cmd)
 {
 	char *shell;
 	pid_t pid;
-	int devnull, status;
+	int status;
 
 	if ((shell = getenv("SHELL")) == NULL)
 		shell = _PATH_BSHELL;
@@ -519,23 +519,14 @@ execute_in_shell(const char *cmd)
 		    shell, strerror(errno));
 	}
 
-	/* Need this to redirect subprocess stdin/out */
-	if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1)
-		fatal("open(/dev/null): %s", strerror(errno));
-
 	debug("Executing command: '%.500s'", cmd);
 
 	/* Fork and execute the command. */
 	if ((pid = fork()) == 0) {
 		char *argv[4];
 
-		/* Redirect child stdin and stdout. Leave stderr */
+		if (stdfd_devnull(1, 1, 0) == -1)
-		if (dup2(devnull, STDIN_FILENO) == -1)
+			fatal("%s: stdfd_devnull failed", __func__);
-			fatal("dup2: %s", strerror(errno));
-		if (dup2(devnull, STDOUT_FILENO) == -1)
-			fatal("dup2: %s", strerror(errno));
-		if (devnull > STDERR_FILENO)
-			close(devnull);
 		closefrom(STDERR_FILENO + 1);
 
 		argv[0] = shell;
@@ -554,8 +545,6 @@ execute_in_shell(const char *cmd)
 	if (pid == -1)
 		fatal("%s: fork: %.100s", __func__, strerror(errno));
 
-	close(devnull);
-
 	while (waitpid(pid, &status, 0) == -1) {
 		if (errno != EINTR && errno != EAGAIN)
 			fatal("%s: waitpid: %s", __func__, strerror(errno));

readpass.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: readpass.c,v 1.63 2020/08/11 09:45:54 djm Exp $ */
+/* $OpenBSD: readpass.c,v 1.64 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  *
@@ -232,7 +232,6 @@ notify_start(int force_askpass, const char *fmt, ...)
 {
 	va_list args;
 	char *prompt = NULL;
-	int devnull;
 	pid_t pid;
 	void (*osigchld)(int);
 	const char *askpass, *s;
@@ -270,11 +269,8 @@ notify_start(int force_askpass, const char *fmt, ...)
 		return NULL;
 	}
 	if (pid == 0) {
-		if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1)
+		if (stdfd_devnull(1, 1, 0) == -1)
-			fatal("%s: open %s", __func__, strerror(errno));
+			fatal("%s: stdfd_devnull failed", __func__);
-		if (dup2(devnull, STDIN_FILENO) == -1 ||
-		    dup2(devnull, STDOUT_FILENO) == -1)
-			fatal("%s: dup2: %s", __func__, strerror(errno));
 		closefrom(STDERR_FILENO + 1);
 		setenv("SSH_ASKPASS_PROMPT", "none", 1); /* hint to UI */
 		execlp(askpass, askpass, prompt, (char *)NULL);

ssh-agent.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.264 2020/09/18 08:16:38 djm Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.265 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1273,7 +1273,7 @@ int
 main(int ac, char **av)
 {
 	int c_flag = 0, d_flag = 0, D_flag = 0, k_flag = 0, s_flag = 0;
-	int sock, fd, ch, result, saved_errno;
+	int sock, ch, result, saved_errno;
 	char *shell, *format, *pidstr, *agentsocket = NULL;
 #ifdef HAVE_SETRLIMIT
 	struct rlimit rlim;
@@ -1493,14 +1493,8 @@ main(int ac, char **av)
 	}
 
 	(void)chdir("/");
-	if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+	if (stdfd_devnull(1, 1, 1) == -1)
-		/* XXX might close listen socket */
+		error("%s: stdfd_devnull failed", __func__);
-		(void)dup2(fd, STDIN_FILENO);
-		(void)dup2(fd, STDOUT_FILENO);
-		(void)dup2(fd, STDERR_FILENO);
-		if (fd > 2)
-			close(fd);
-	}
 
 #ifdef HAVE_SETRLIMIT
 	/* deny core dumps, since memory contains unencrypted private keys */

ssh.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.c,v 1.536 2020/09/21 07:29:09 djm Exp $ */
+/* $OpenBSD: ssh.c,v 1.537 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1698,7 +1698,6 @@ static void
 control_persist_detach(void)
 {
 	pid_t pid;
-	int devnull, keep_stderr;
 
 	debug("%s: backgrounding master process", __func__);
 
@@ -1725,18 +1724,8 @@ control_persist_detach(void)
 		/* muxclient() doesn't return on success. */
 		fatal("Failed to connect to new control master");
 	}
-	if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
+	if (stdfd_devnull(1, 1, !(log_is_on_stderr() && debug_flag)) == -1)
-		error("%s: open(\"/dev/null\"): %s", __func__,
+		error("%s: stdfd_devnull failed", __func__);
-		    strerror(errno));
-	} else {
-		keep_stderr = log_is_on_stderr() && debug_flag;
-		if (dup2(devnull, STDIN_FILENO) == -1 ||
-		    dup2(devnull, STDOUT_FILENO) == -1 ||
-		    (!keep_stderr && dup2(devnull, STDERR_FILENO) == -1))
-			error("%s: dup2: %s", __func__, strerror(errno));
-		if (devnull > STDERR_FILENO)
-			close(devnull);
-	}
 	daemon(1, 1);
 	setproctitle("%s [mux]", options.control_path);
 }
@@ -1745,26 +1734,14 @@ control_persist_detach(void)
 static void
 fork_postauth(void)
 {
-	int devnull, keep_stderr;
-
 	if (need_controlpersist_detach)
 		control_persist_detach();
 	debug("forking to background");
 	fork_after_authentication_flag = 0;
 	if (daemon(1, 1) == -1)
 		fatal("daemon() failed: %.200s", strerror(errno));
-	if ((devnull = open(_PATH_DEVNULL, O_WRONLY)) == -1)
+	if (stdfd_devnull(1, 1, !(log_is_on_stderr() && debug_flag)) == -1)
-		error("%s: open %s: %s", __func__,
+		error("%s: stdfd_devnull failed", __func__);
-		    _PATH_DEVNULL, strerror(errno));
-	else {
-		keep_stderr = log_is_on_stderr() && debug_flag;
-		if (dup2(devnull, STDIN_FILENO) == -1 ||
-		    dup2(devnull, STDOUT_FILENO) == -1 ||
-		    (!keep_stderr && dup2(devnull, STDOUT_FILENO) == -1))
-			fatal("%s: dup2() stdio failed", __func__);
-		if (devnull > STDERR_FILENO)
-			close(devnull);
-	}
 }
 
 static void
@@ -2075,7 +2052,7 @@ ssh_session2_open(struct ssh *ssh)
 static int
 ssh_session2(struct ssh *ssh, struct passwd *pw)
 {
-	int r, devnull, id = -1;
+	int r, id = -1;
 	char *cp, *tun_fwd_ifname = NULL;
 
 	/* XXX should be pre-session */
@@ -2162,17 +2139,8 @@ ssh_session2(struct ssh *ssh, struct passwd *pw)
 	 * NB. this can only happen after LocalCommand has completed,
 	 * as it may want to write to stdout.
 	 */
-	if (!need_controlpersist_detach) {
+	if (!need_controlpersist_detach && stdfd_devnull(0, 1, 0) == -1)
-		if ((devnull = open(_PATH_DEVNULL, O_WRONLY)) == -1) {
+		error("%s: stdfd_devnull failed", __func__);
-			error("%s: open %s: %s", __func__,
-			    _PATH_DEVNULL, strerror(errno));
-		} else {
-			if (dup2(devnull, STDOUT_FILENO) == -1)
-				fatal("%s: dup2() stdout failed", __func__);
-			if (devnull > STDERR_FILENO)
-				close(devnull);
-		}
-	}
 
 	/*
 	 * If requested and we are not interested in replies to remote

sshconnect.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: sshconnect.c,v 1.333 2020/10/03 08:11:28 djm Exp $ */
+/* $OpenBSD: sshconnect.c,v 1.334 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -105,24 +105,6 @@ expand_proxy_command(const char *proxy_command, const char *user,
 	return ret;
 }
 
-static void
-stderr_null(void)
-{
-	int devnull;
-
-	if ((devnull = open(_PATH_DEVNULL, O_WRONLY)) == -1) {
-		error("Can't open %s for stderr redirection: %s",
-		    _PATH_DEVNULL, strerror(errno));
-		return;
-	}
-	if (devnull == STDERR_FILENO)
-		return;
-	if (dup2(devnull, STDERR_FILENO) == -1)
-		error("Cannot redirect stderr to %s", _PATH_DEVNULL);
-	if (devnull > STDERR_FILENO)
-		close(devnull);
-}
-
 /*
  * Connect to the given ssh server using a proxy command that passes a
  * a connected fd back to us.
@@ -169,8 +151,8 @@ ssh_proxy_fdpass_connect(struct ssh *ssh, const char *host,
 		 * error messages may be printed on the user's terminal.
 		 */
 		if (!debug_flag && options.control_path != NULL &&
-		    options.control_persist)
+		    options.control_persist && stdfd_devnull(0, 0, 1) == -1)
-			stderr_null();
+			error("%s: stdfd_devnull failed", __func__);
 
 		argv[0] = shell;
 		argv[1] = "-c";
@@ -252,8 +234,8 @@ ssh_proxy_connect(struct ssh *ssh, const char *host, const char *host_arg,
 		 * error messages may be printed on the user's terminal.
 		 */
 		if (!debug_flag && options.control_path != NULL &&
-		    options.control_persist)
+		    options.control_persist && stdfd_devnull(0, 0, 1) == -1)
-			stderr_null();
+			error("%s: stdfd_devnull failed", __func__);
 
 		argv[0] = shell;
 		argv[1] = "-c";

sshd.c

@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.561 2020/08/27 01:06:19 djm Exp $ */
+/* $OpenBSD: sshd.c,v 1.562 2020/10/03 09:22:26 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1024,8 +1024,6 @@ recv_rexec_state(int fd, struct sshbuf *conf)
 static void
 server_accept_inetd(int *sock_in, int *sock_out)
 {
-	int fd;
-
 	if (rexeced_flag) {
 		close(REEXEC_CONFIG_PASS_FD);
 		*sock_in = *sock_out = dup(STDIN_FILENO);
@@ -1038,14 +1036,8 @@ server_accept_inetd(int *sock_in, int *sock_out)
 	 * as our code for setting the descriptors won't work if
 	 * ttyfd happens to be one of those.
 	 */
-	if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+	if (stdfd_devnull(1, 1, !log_stderr) == -1)
-		dup2(fd, STDIN_FILENO);
+		error("%s: stdfd_devnull failed", __func__);
-		dup2(fd, STDOUT_FILENO);
-		if (!log_stderr)
-			dup2(fd, STDERR_FILENO);
-		if (fd > (log_stderr ? STDERR_FILENO : STDOUT_FILENO))
-			close(fd);
-	}
 	debug("inetd sockets after dupping: %d, %d", *sock_in, *sock_out);
 }
 
@@ -2092,8 +2084,6 @@ main(int ac, char **av)
 #endif
 
 	if (rexec_flag) {
-		int fd;
-
 		debug("rexec start in %d out %d newsock %d pipe %d sock %d",
 		    sock_in, sock_out, newsock, startup_pipe, config_s[0]);
 		dup2(newsock, STDIN_FILENO);
@@ -2121,12 +2111,8 @@ main(int ac, char **av)
 		/* Clean up fds */
 		close(REEXEC_CONFIG_PASS_FD);
 		newsock = sock_out = sock_in = dup(STDIN_FILENO);
-		if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+		if (stdfd_devnull(1, 1, 0) == -1)
-			dup2(fd, STDIN_FILENO);
+			error("%s: stdfd_devnull failed", __func__);
-			dup2(fd, STDOUT_FILENO);
-			if (fd > STDERR_FILENO)
-				close(fd);
-		}
 		debug("rexec cleanup in %d out %d newsock %d pipe %d sock %d",
 		    sock_in, sock_out, newsock, startup_pipe, config_s[0]);
 	}