From: millert <millert@openbsd.org>
Date: Tue, 10 Jan 2023 23:22:15 +0000 (+0000)
Subject: Switch scp from using pipes to a socketpair for communication with
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=0903cde6a22bc25f089d23dd1d3806894b9b558b;p=openbsd

Switch scp from using pipes to a socketpair for communication with
it's ssh sub-processes.  We no longer need to reserve two descriptors
to ensure that we don't end up using fd 0-2 unexpectedly, that is
handled by sanitise_stdfd() in main().
Based on an original diff from djm@.  OK deraadt@ djm@
---

diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c
index 9946ff1692e..6e0f5f8de92 100644
--- a/usr.bin/ssh/scp.c
+++ b/usr.bin/ssh/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.251 2022/12/16 06:52:48 jmc Exp $ */
+/* $OpenBSD: scp.c,v 1.252 2023/01/10 23:22:15 millert Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -251,7 +251,7 @@ int
 do_cmd(char *program, char *host, char *remuser, int port, int subsystem,
     char *cmd, int *fdin, int *fdout, pid_t *pid)
 {
-	int pin[2], pout[2], reserved[2];
+	int sv[2];
 
 	if (verbose_mode)
 		fmprintf(stderr,
@@ -262,22 +262,9 @@ do_cmd(char *program, char *host, char *remuser, int port, int subsystem,
 	if (port == -1)
 		port = sshport;
 
-	/*
-	 * Reserve two descriptors so that the real pipes won't get
-	 * descriptors 0 and 1 because that will screw up dup2 below.
-	 */
-	if (pipe(reserved) == -1)
-		fatal("pipe: %s", strerror(errno));
-
 	/* Create a socket pair for communicating with ssh. */
-	if (pipe(pin) == -1)
-		fatal("pipe: %s", strerror(errno));
-	if (pipe(pout) == -1)
-		fatal("pipe: %s", strerror(errno));
-
-	/* Free the reserved descriptors. */
-	close(reserved[0]);
-	close(reserved[1]);
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) == -1)
+		fatal("socketpair: %s", strerror(errno));
 
 	ssh_signal(SIGTSTP, suspchild);
 	ssh_signal(SIGTTIN, suspchild);
@@ -285,15 +272,18 @@ do_cmd(char *program, char *host, char *remuser, int port, int subsystem,
 
 	/* Fork a child to execute the command on the remote host using ssh. */
 	*pid = fork();
-	if (*pid == 0) {
+	switch (*pid) {
+	case -1:
+		fatal("fork: %s", strerror(errno));
+	case 0:
 		/* Child. */
-		close(pin[1]);
-		close(pout[0]);
-		dup2(pin[0], 0);
-		dup2(pout[1], 1);
-		close(pin[0]);
-		close(pout[1]);
-
+		if (dup2(sv[0], STDIN_FILENO) == -1 ||
+		    dup2(sv[0], STDOUT_FILENO) == -1) {
+			perror("dup2");
+			_exit(1);
+		}
+		close(sv[0]);
+		close(sv[1]);
 		replacearg(&args, 0, "%s", program);
 		if (port != -1) {
 			addargs(&args, "-p");
@@ -311,19 +301,17 @@ do_cmd(char *program, char *host, char *remuser, int port, int subsystem,
 
 		execvp(program, args.list);
 		perror(program);
-		exit(1);
-	} else if (*pid == -1) {
-		fatal("fork: %s", strerror(errno));
+		_exit(1);
+	default:
+		/* Parent.  Close the other side, and return the local side. */
+		close(sv[0]);
+		*fdin = sv[1];
+		*fdout = sv[1];
+		ssh_signal(SIGTERM, killchild);
+		ssh_signal(SIGINT, killchild);
+		ssh_signal(SIGHUP, killchild);
+		return 0;
 	}
-	/* Parent.  Close the other side, and return the local side. */
-	close(pin[0]);
-	*fdout = pin[1];
-	close(pout[1]);
-	*fdin = pout[0];
-	ssh_signal(SIGTERM, killchild);
-	ssh_signal(SIGINT, killchild);
-	ssh_signal(SIGHUP, killchild);
-	return 0;
 }
 
 /*