- markus@cvs.openbsd.org 2012/06/30 14:35:09

     [sandbox-systrace.c sshd.c]
     fix a during the load of the sandbox policies (child can still make
     the read-syscall and wait forever for systrace-answers) by replacing
     the read/write synchronisation with SIGSTOP/SIGCONT;
     report and help hshoexer@; ok djm@, dtucker@

3 files changed, 35 insertions, 30 deletions

diff --git a/ChangeLog b/ChangeLog
index 5608909da..66c4ef574 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,12 @@
      [ssh_config.5 sshd_config.5]
      match the documented MAC order of preference to the actual one;
      ok dtucker@
+   - markus@cvs.openbsd.org 2012/06/30 14:35:09
+     [sandbox-systrace.c sshd.c]
+     fix a during the load of the sandbox policies (child can still make
+     the read-syscall and wait forever for systrace-answers) by replacing
+     the read/write synchronisation with SIGSTOP/SIGCONT;
+     report and help hshoexer@; ok djm@, dtucker@
 
 20120629
  - OpenBSD CVS Sync
diff --git a/sandbox-systrace.c b/sandbox-systrace.c
index 199b69f44..2d16a627f 100644
--- a/sandbox-systrace.c
+++ b/sandbox-systrace.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sandbox-systrace.c,v 1.5 2012/06/26 11:02:30 dtucker Exp $ */
+/* $OpenBSD: sandbox-systrace.c,v 1.6 2012/06/30 14:35:09 markus Exp $ */
 /*
  * Copyright (c) 2011 Damien Miller <djm@mindrot.org>
  *
@@ -24,12 +24,14 @@
 #include <sys/ioctl.h>
 #include <sys/syscall.h>
 #include <sys/socket.h>
+#include <sys/wait.h>
 
 #include <dev/systrace.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <signal.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -69,26 +71,21 @@ static const struct sandbox_policy preauth_policy[] = {
 };
 
 struct ssh_sandbox {
-        int child_sock;
-        int parent_sock;
         int systrace_fd;
         pid_t child_pid;
+        void (*osigchld)(int);
 };
 
 struct ssh_sandbox *
 ssh_sandbox_init(void)
 {
         struct ssh_sandbox *box;
-        int s[2];
 
         debug3("%s: preparing systrace sandbox", __func__);
         box = xcalloc(1, sizeof(*box));
-        if (socketpair(AF_UNIX, SOCK_STREAM, 0, s) == -1)
-                fatal("%s: socketpair: %s", __func__, strerror(errno));
-        box->child_sock = s[0];
-        box->parent_sock = s[1];
         box->systrace_fd = -1;
         box->child_pid = 0;
+        box->osigchld = signal(SIGCHLD, SIG_IGN);
 
         return box;
 }
@@ -96,35 +93,38 @@ ssh_sandbox_init(void)
 void
 ssh_sandbox_child(struct ssh_sandbox *box)
 {
-        char whatever = 0;
-
-        close(box->parent_sock);
-        /* Signal parent that we are ready */
         debug3("%s: ready", __func__);
-        if (atomicio(vwrite, box->child_sock, &whatever, 1) != 1)
-                fatal("%s: write: %s", __func__, strerror(errno));
-        /* Wait for parent to signal for us to go */
-        if (atomicio(read, box->child_sock, &whatever, 1) != 1)
-                fatal("%s: read: %s", __func__, strerror(errno));
+        signal(SIGCHLD, box->osigchld);
+        if (kill(getpid(), SIGSTOP) != 0)
+                fatal("%s: kill(%d, SIGSTOP)", __func__, getpid());
         debug3("%s: started", __func__);
-        close(box->child_sock);
 }
 
 static void
 ssh_sandbox_parent(struct ssh_sandbox *box, pid_t child_pid,
     const struct sandbox_policy *allowed_syscalls)
 {
-        int dev_systrace, i, j, found;
-        char whatever = 0;
+        int dev_systrace, i, j, found, status;
+        pid_t pid;
         struct systrace_policy policy;
 
+        /* Wait for the child to send itself a SIGSTOP */
         debug3("%s: wait for child %ld", __func__, (long)child_pid);
+        do {
+                pid = waitpid(child_pid, &status, WUNTRACED);
+        } while (pid == -1 && errno == EINTR);
+        signal(SIGCHLD, box->osigchld);
+        if (!WIFSTOPPED(status)) {
+                if (WIFSIGNALED(status))
+                        fatal("%s: child terminated with signal %d",
+                            __func__, WTERMSIG(status));
+                if (WIFEXITED(status))
+                        fatal("%s: child exited with status %d",
+                            __func__, WEXITSTATUS(status));
+                fatal("%s: child not stopped", __func__);
+        }
+        debug3("%s: child %ld stopped", __func__, (long)child_pid);
         box->child_pid = child_pid;
-        close(box->child_sock);
-        /* Wait for child to signal that it is ready */
-        if (atomicio(read, box->parent_sock, &whatever, 1) != 1)
-                fatal("%s: read: %s", __func__, strerror(errno));
-        debug3("%s: child %ld ready", __func__, (long)child_pid);
 
         /* Set up systracing of child */
         if ((dev_systrace = open("/dev/systrace", O_RDONLY)) == -1)
@@ -175,9 +175,8 @@ ssh_sandbox_parent(struct ssh_sandbox *box, pid_t child_pid,
 
         /* Signal the child to start running */
         debug3("%s: start child %ld", __func__, (long)child_pid);
-        if (atomicio(vwrite, box->parent_sock, &whatever, 1) != 1)
-                fatal("%s: write: %s", __func__, strerror(errno));
-        close(box->parent_sock);
+        if (kill(box->child_pid, SIGCONT) != 0)
+                fatal("%s: kill(%d, SIGCONT)", __func__, box->child_pid);
 }
 
 void
diff --git a/sshd.c b/sshd.c
index 7cc7044be..64b846f6c 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.391 2012/05/13 01:42:32 dtucker Exp $ */
+/* $OpenBSD: sshd.c,v 1.392 2012/06/30 14:35:09 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -643,9 +643,9 @@ privsep_preauth(Authctxt *authctxt)
         } else if (pid != 0) {
                 debug2("Network child is on pid %ld", (long)pid);
 
+                pmonitor->m_pid = pid;
                 if (box != NULL)
                         ssh_sandbox_parent_preauth(box, pid);
-                pmonitor->m_pid = pid;
                 monitor_child_preauth(authctxt, pmonitor);
 
                 /* Sync memory */