Switch scp from using pipes to a socketpair for communication with
authormillert <millert@openbsd.org>
Tue, 10 Jan 2023 23:22:15 +0000 (23:22 +0000)
committermillert <millert@openbsd.org>
Tue, 10 Jan 2023 23:22:15 +0000 (23:22 +0000)
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@

usr.bin/ssh/scp.c

index 9946ff1..6e0f5f8 100644 (file)
@@ -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;
 }
 
 /*