2005-11-23 16:00:00 PST	Dongmin Zhang <zhangdm at google.com>, Dan Kegel <dank at kegel.com>, Thomas Kho <tkho at google.com>

    Summary:
      Kill compile jobs if the client disconnects.
      Also kill them if they go on longer than 30 minutes.
      This protects us against strange networking problems and infinite loops in compilers.
      Disadvantage: since this uses waitpid instead of wait4, it no longer reports child time on completion :-(

    Modified files:
      src/compile.c src/exec.c src/exec.h src/prefork.c src/remote.c src/serve.c


diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/man/distccd.1 distcc-2.18.3-timeout/man/distccd.1
--- distcc-2.18.3-pristine/man/distccd.1	2005-11-08 11:59:47.000000000 -0800
+++ distcc-2.18.3-timeout/man/distccd.1	2005-12-08 13:16:29.414985000 -0800
@@ -147,6 +147,11 @@
 connections are rejected by closing the TCP connection immediately.  A
 warning is logged on the server but nothing is sent ot the client.
 .TP
+.B --job-lifetime SECONDS
+Kills a distccd job if it runs for more than SECONDS seconds. This prevents
+denial of service from clients that don't properly disconnect and compilers
+that fail to terminate. By default this is turned off.
+.TP
 .B --no-detach
 Do not detach from the shell that started the daemon.  
 .TP
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/compile.c distcc-2.18.3-timeout/src/compile.c
--- distcc-2.18.3-pristine/src/compile.c	2005-11-08 11:59:51.000000000 -0800
+++ distcc-2.18.3-timeout/src/compile.c	2005-12-08 10:34:33.265491000 -0800
@@ -77,7 +77,7 @@
     if ((ret = dcc_spawn_child(argv, &pid, NULL, NULL, NULL)) != 0)
         return ret;
 
-    if ((ret = dcc_collect_child("cc", pid, &status)))
+    if ((ret = dcc_collect_child("cc", pid, &status, timeout_null_fd)))
         return ret;
 
     return dcc_critique_status(status, "compile", input_name,
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/dopt.c distcc-2.18.3-timeout/src/dopt.c
--- distcc-2.18.3-pristine/src/dopt.c	2005-11-08 11:59:54.000000000 -0800
+++ distcc-2.18.3-timeout/src/dopt.c	2005-12-08 13:33:22.930457000 -0800
@@ -44,6 +44,7 @@
 #include "exitcode.h"
 #include "daemon.h"
 #include "access.h"
+#include "exec.h"
 
 int opt_niceness = 5;           /* default */
 
@@ -86,6 +87,8 @@
 const char *arg_pid_file = NULL;
 const char *arg_log_file = NULL;
 
+int opt_job_lifetime = 0;
+
 /* Enumeration values for options that don't have single-letter name.  These
  * must be numerically above all the ascii letters. */
 enum {
@@ -105,6 +108,7 @@
     { "log-file", 0,     POPT_ARG_STRING, &arg_log_file, 0, 0, 0 },
     { "log-level", 0,    POPT_ARG_STRING, 0, opt_log_level, 0, 0 },
     { "log-stderr", 0,   POPT_ARG_NONE, &opt_log_stderr, 0, 0, 0 },
+    { "job-lifetime", 0, POPT_ARG_INT, &opt_job_lifetime, 'l', 0, 0 },
     { "nice", 'N',       POPT_ARG_INT,  &opt_niceness,  0, 0, 0 },
     { "no-detach", 0,    POPT_ARG_NONE, &opt_no_detach, 0, 0, 0 },
     { "no-fifo", 0,      POPT_ARG_NONE, &opt_no_fifo, 0, 0, 0 },
@@ -133,6 +137,7 @@
 "    -N, --nice LEVEL           lower priority, 20=most nice\n"
 "    --user USER                if run by root, change to this persona\n"
 "    --jobs, -j LIMIT           maximum tasks at any time\n"
+"    --job-lifetime SECONDS     maximum lifetime of a compile request\n"
 "  Networking:\n"
 "    -p, --port PORT            TCP port to listen on\n"
 "    --listen ADDRESS           IP address to listen on\n"
@@ -196,6 +201,13 @@
             }
             break;
 
+        case 'l':
+            if (opt_job_lifetime < 0) {
+                opt_job_lifetime = 0;
+            }
+            dcc_job_lifetime = opt_job_lifetime;
+            break;
+
         case 'u':
             if (getuid() != 0 && geteuid() != 0) {
                 rs_log_warning("--user is ignored when distccd is not run by root");
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/dopt.h distcc-2.18.3-timeout/src/dopt.h
--- distcc-2.18.3-pristine/src/dopt.h	2005-11-08 11:59:54.000000000 -0800
+++ distcc-2.18.3-timeout/src/dopt.h	2005-12-08 13:21:57.124947000 -0800
@@ -32,6 +32,7 @@
 extern int opt_no_prefork;
 extern int opt_no_detach;
 extern int opt_daemon_mode, opt_inetd_mode;
+extern int opt_job_lifetime;
 extern const char *arg_log_file;
 extern int opt_no_fifo;
 extern int opt_log_stderr;
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/exec.c distcc-2.18.3-timeout/src/exec.c
--- distcc-2.18.3-pristine/src/exec.c	2005-11-08 11:59:54.000000000 -0800
+++ distcc-2.18.3-timeout/src/exec.c	2005-12-08 13:33:47.598825000 -0800
@@ -57,6 +57,7 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/resource.h>
+#include <sys/poll.h>
 
 #include "distcc.h"
 #include "trace.h"
@@ -65,6 +66,10 @@
 #include "exec.h"
 #include "lock.h"
 #include "hosts.h"
+#include "dopt.h"
+
+const int timeout_null_fd = -1;
+int dcc_job_lifetime = 0;
 
 static void dcc_inside_child(char **argv,
                              const char *stdin_file,
@@ -242,6 +247,17 @@
         rs_log_error("failed to fork: %s", strerror(errno));
         return EXIT_OUT_OF_MEMORY; /* probably */
     } else if (pid == 0) {
+    	/* If this is a remote compile,
+	 * put the child in a new group, so we can
+	 * kill it and all its descendents without killing distccd 
+	 * FIXME: if you kill distccd while it's compiling, and
+	 * the compiler has an infinite loop bug, the new group
+	 * will run forever until you kill it.
+	 */
+	if (stdout_file != NULL) {
+	    if (dcc_new_pgrp() != 0)
+	        rs_trace("Unable to start a new group\n");
+	}
         dcc_inside_child(argv, stdin_file, stdout_file, stderr_file);
         /* !! NEVER RETURN FROM HERE !! */
     } else {
@@ -266,13 +282,15 @@
 
 static int sys_wait4(pid_t pid, int *status, int options, struct rusage *rusage)
 {
-#ifdef HAVE_WAIT4
-    return wait4(pid, status, options, rusage);
-#elif HAVE_WAITPID
+
+    /* Prefer use waitpid to wait4 for non-blocking wait with WNOHANG option */
+#ifdef HAVE_WAITPID
     /* Just doing getrusage(children) is not sufficient, because other
      * children may have exited previously. */
     memset(rusage, 0, sizeof *rusage);
     return waitpid(pid, status, options);
+#elif HAVE_WAIT4
+    return wait4(pid, status, options, rusage);
 #else
 #error Please port this
 #endif
@@ -289,18 +307,45 @@
  * waits all the time.
  **/
 int dcc_collect_child(const char *what, pid_t pid,
-                      int *wait_status)
+                      int *wait_status, int in_fd)
 {
     struct rusage ru;
     pid_t ret_pid;
+
+    int ret;
+    int wait_timeout_sec;
+    fd_set fds,readfds;
+   
+    wait_timeout_sec = dcc_job_lifetime;
+	   
+    FD_ZERO(&readfds);
+    if (in_fd != timeout_null_fd){ 
+        FD_SET(in_fd,&readfds);
+    }
+	    
     
-    while (1) {
-        if ((ret_pid = sys_wait4(pid, wait_status, 0, &ru)) != -1) {
-            /* This is not the main user-visible message, that comes from
-             * critique_status(). */
+    while (!dcc_job_lifetime || wait_timeout_sec-- >= 0) {
+
+	/* If we're called with a socket, break out of the loop if the socket disconnects.
+	 * To do that, we need to block in select, not in sys_wait4.
+	 * (Only waitpid uses WNOHANG to mean don't block ever, so I've modified
+	 *  sys_wait4 above to preferentially call waitpid.)
+	 */
+	int flags = (in_fd == timeout_null_fd) ? 0 : WNOHANG;
+        ret_pid = sys_wait4(pid, wait_status, flags, &ru);
+
+	if (ret_pid == -1) {
+	    if (errno == EINTR) {
+		rs_trace("wait4 was interrupted; retrying");
+	    } else {
+		rs_log_error("sys_wait4(pid=%d) borked: %s", (int) pid, strerror(errno));
+		return EXIT_DISTCC_FAILED;
+	    }
+        } else if (ret_pid != 0) {
+            /* This is not the main user-visible message; that comes from
+              * critique_status(). */
             rs_trace("%s child %ld terminated with status %#x",
                      what, (long) ret_pid, *wait_status);
-
             rs_log_info("%s times: user %ld.%06lds, system %ld.%06lds, "
                         "%ld minflt, %ld majflt",
                         what,
@@ -309,14 +354,46 @@
                         ru.ru_minflt, ru.ru_majflt);
 
             return 0;
-        } else if (errno == EINTR) {
-            rs_trace("wait4 was interrupted; retrying");
-            continue;
-        } else {
-            rs_log_error("sys_wait4(pid=%d) borked: %s", (int) pid, strerror(errno));
-            return EXIT_DISTCC_FAILED;
-        }
+	}
+
+	/* check timeout */
+	if (in_fd != timeout_null_fd){
+            struct timeval timeout;
+
+	    /* If client disconnects, the socket will become readable,
+	     * and a read should return -1 and set errno to EPIPE. 
+	     */
+	    fds = readfds;
+            timeout.tv_sec = 1;
+            timeout.tv_usec = 0;
+	    ret = select(in_fd+1,&fds,NULL,NULL,&timeout);
+	    if (ret == 1) {
+		char buf;
+		int nread = read(in_fd, &buf, 1);
+		if ((nread == -1) && (errno == EWOULDBLOCK)) {
+		    /* spurious wakeup, ignore */
+		    ;
+		} else if (nread == 0) {
+		    rs_log_error("Client fd disconnected, killing job");
+		    /* If killpg fails, it might means the child process is not 
+		     * in a new group, so, just kill the child process */
+		    if (killpg(pid,SIGTERM)!=0)
+			kill(pid, SIGTERM);
+		    return EXIT_DISTCC_FAILED;
+		} else if (nread == 1) {
+		    rs_log_error("Bug!  Read from fd succeeded when checking whether client disconnected!");
+		} else 
+		    rs_log_error("Bug!  nread %d, errno %d checking whether client disconnected!", nread, errno);
+	    }	
+	} else
+	    poll(NULL,NULL,1000);
     }
+    /* If timeout, also kill the child process */
+    if (killpg(pid,SIGTERM) !=0 )
+        kill(pid, SIGTERM);
+    rs_log_error("Compilation takes too long, timeout.");
+
+    return EXIT_DISTCC_FAILED;
 }
 
 
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/exec.h distcc-2.18.3-timeout/src/exec.h
--- distcc-2.18.3-pristine/src/exec.h	2005-11-08 11:59:54.000000000 -0800
+++ distcc-2.18.3-timeout/src/exec.h	2005-12-08 13:32:31.216249000 -0800
@@ -21,6 +21,9 @@
  */
 
 /* exec.c */
+extern const int timeout_null_fd;
+extern int dcc_job_lifetime;
+
 int dcc_redirect_fds(const char *stdin_file,
                      const char *stdout_file,
                      const char *stderr_file);
@@ -28,8 +31,9 @@
 int dcc_spawn_child(char **argv, pid_t *pidptr,
                     const char *, const char *, const char *);
 
+/* if in_fd is timeout_null_fd, means this parameter is not used */
 int dcc_collect_child(const char *what, pid_t pid,
-                      int *wait_status);
+                      int *wait_status, int in_fd);
 int dcc_critique_status(int s,
                         const char *,
                         const char *,
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/prefork.c distcc-2.18.3-timeout/src/prefork.c
--- distcc-2.18.3-pristine/src/prefork.c	2005-11-08 11:59:52.000000000 -0800
+++ distcc-2.18.3-timeout/src/prefork.c	2005-12-08 13:32:57.160251000 -0800
@@ -56,19 +56,6 @@
 
 static int dcc_preforked_child(int listen_fd);
 
-/*
- * TODO: It might be nice if killing a child, or dropping the socket caused
- * its compiler to be killed too.  This would probably require running the
- * compiler in a separate process group, and remembering to kill it when the
- * child is signalled.
- *
- * I can only think of a few cases where this would be a big deal: a runaway
- * compiler, where you probably can just manually kill that process, and a
- * client interrupt, in which case allowing the compiler to run to completion
- * is not such a big deal.
- */
-     
-
 /**
  * Main loop for the parent process with the new preforked implementation.
  * The parent is just responsible for keeping a pool of children and they
@@ -127,11 +114,23 @@
         
         cli_len = sizeof cli_addr;
 
+	/* Cancel any previously scheduled alarm */
+        if (dcc_job_lifetime)
+            alarm(0);
+
         do {
             acc_fd = accept(listen_fd, (struct sockaddr *) &cli_addr,
                             &cli_len);
         } while (acc_fd == -1 && errno == EINTR);
 
+        /* Kill this process if the compile job takes too long.
+         * The synchronous timeout should happen first, so this alarm
+         * should fire only if the client stops transferring network data without disconnecting.
+         */
+        if (dcc_job_lifetime)
+            alarm(dcc_job_lifetime+30);
+
+
         if (acc_fd == -1) {
             rs_log_error("accept failed: %s", strerror(errno));
             dcc_exit(EXIT_CONNECT_FAILED);
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/remote.c distcc-2.18.3-timeout/src/remote.c
--- distcc-2.18.3-pristine/src/remote.c	2005-11-08 11:59:50.000000000 -0800
+++ distcc-2.18.3-timeout/src/remote.c	2005-12-08 10:34:33.281475000 -0800
@@ -102,7 +102,7 @@
         /* Wait for cpp to finish (if not already done), check the
          * result, then send the .i file */
         
-        if ((ret = dcc_collect_child("cpp", cpp_pid, status)))
+        if ((ret = dcc_collect_child("cpp", cpp_pid, status, timeout_null_fd)))
             return ret;
 
         /* Although cpp failed, there is no need to try running the command
@@ -256,7 +256,7 @@
      * when something possibly went wrong, and it allows us to account for the
      * cost of the ssh child. */
     if (ssh_pid) {
-        dcc_collect_child("ssh", ssh_pid, &ssh_status); /* ignore failure */
+        dcc_collect_child("ssh", ssh_pid, &ssh_status, timeout_null_fd); /* ignore failure */
     }
     
     return ret;
diff -Naur -x '*.o' -x '*.log' -x '*.status' -x distcc -x distccd -x .stamp-conf -x '*config.h' -x '*tags' -x '*Makefile' -x '*distccmon-text' distcc-2.18.3-pristine/src/serve.c distcc-2.18.3-timeout/src/serve.c
--- distcc-2.18.3-pristine/src/serve.c	2005-11-08 11:59:52.000000000 -0800
+++ distcc-2.18.3-timeout/src/serve.c	2005-12-08 10:34:33.285485000 -0800
@@ -306,7 +306,7 @@
 
     if ((compile_ret = dcc_spawn_child(argv, &cc_pid,
                                        "/dev/null", out_fname, err_fname))
-        || (compile_ret = dcc_collect_child("cc", cc_pid, &status))) {
+        || (compile_ret = dcc_collect_child("cc", cc_pid, &status, in_fd))) {
         /* We didn't get around to finding a wait status from the actual compiler */
         status = W_EXITCODE(compile_ret, 0);
     }
