[core] consolidate fork()/execve() code (#1393)

(refactoring work to address issue #1393)

x-ref:
  "access log pipe writer should restart child process if it exits"
  https://redmine.lighttpd.net/issues/1393

src/fdevent.c

@@ -373,6 +373,31 @@ int fdevent_open_cloexec(const char *pathname, int flags, mode_t mode) {
 }
 
 
+int fdevent_open_devnull(void) {
+  #if defined(_WIN32)
+    return fdevent_open_cloexec("nul", O_RDWR, 0);
+  #else
+    return fdevent_open_cloexec("/dev/null", O_RDWR, 0);
+  #endif
+}
+
+
+int fdevent_open_dirname(char *path) {
+    /*(handle special cases of no dirname or dirname is root directory)*/
+    char * const c = strrchr(path, '/');
+    const char * const dname = (NULL != c ? c == path ? "/" : path : ".");
+    int dfd;
+    int flags = O_RDONLY;
+  #ifdef O_DIRECTORY
+    flags |= O_DIRECTORY;
+  #endif
+    if (NULL != c) *c = '\0';
+    dfd = fdevent_open_cloexec(dname, flags, 0);
+    if (NULL != c) *c = '/';
+    return dfd;
+}
+
+
 int fdevent_accept_listenfd(int listenfd, struct sockaddr *addr, size_t *addrlen) {
 	int fd;
 	socklen_t len = (socklen_t) *addrlen;
@@ -401,6 +426,174 @@ int fdevent_event_next_fdndx(fdevents *ev, int ndx) {
 }
 
 
+#ifdef FD_CLOEXEC
+static int fdevent_dup2_close_clrfd_cloexec(int oldfd, int newfd) {
+    if (oldfd >= 0) {
+        if (oldfd != newfd) {
+            force_assert(oldfd > STDERR_FILENO);
+            if (newfd != dup2(oldfd, newfd)) return -1;
+        }
+        else {
+            fdevent_clrfd_cloexec(newfd);
+        }
+    }
+    return newfd;
+}
+#else
+static int fdevent_dup2_close_clrfd_cloexec(int oldfd, int newfd, int reuse) {
+    if (oldfd >= 0) {
+        if (oldfd != newfd) {
+            force_assert(oldfd > STDERR_FILENO);
+            if (newfd != dup2(oldfd, newfd)) return -1;
+            if (!reuse) close(oldfd);
+        }
+    }
+    return newfd;
+}
+#endif
+
+
+int fdevent_set_stdin_stdout_stderr(int fdin, int fdout, int fderr) {
+  #ifdef FD_CLOEXEC
+    if (STDIN_FILENO != fdevent_dup2_close_clrfd_cloexec(fdin, STDIN_FILENO))
+        return -1;
+    if (STDOUT_FILENO != fdevent_dup2_close_clrfd_cloexec(fdout, STDOUT_FILENO))
+        return -1;
+    if (STDERR_FILENO != fdevent_dup2_close_clrfd_cloexec(fderr, STDERR_FILENO))
+        return -1;
+  #else
+    if (STDIN_FILENO != fdevent_dup2_close_clrfd_cloexec(fdin, STDIN_FILENO,
+                                                         fdin == fdout
+                                                         || fdin == fderr))
+        return -1;
+    if (STDOUT_FILENO != fdevent_dup2_close_clrfd_cloexec(fdout, STDOUT_FILENO,
+                                                          fdout == fderr))
+        return -1;
+    if (STDERR_FILENO != fdevent_dup2_close_clrfd_cloexec(fderr, STDERR_FILENO,
+                                                          0))
+        return -1;
+  #endif
+
+    return 0;
+}
+
+
+#include <stdio.h>      /* perror() */
+#include <signal.h>     /* signal() */
+
+pid_t fdevent_fork_execve(const char *name, char *argv[], char *envp[], int fdin, int fdout, int fderr, int dfd) {
+ #ifdef HAVE_FORK
+
+    pid_t pid = fork();
+    if (0 != pid) return pid; /* parent (pid > 0) or fork() error (-1 == pid) */
+
+    /* child (0 == pid) */
+
+    if (-1 != dfd) {
+        if (0 != fchdir(dfd))
+            _exit(errno);
+        close(dfd);
+    }
+
+    if (0 != fdevent_set_stdin_stdout_stderr(fdin, fdout, fderr)) _exit(errno);
+  #ifdef FD_CLOEXEC
+    /*(might not be sufficient for open fds, but modern OS have FD_CLOEXEC)*/
+    for (int i = 3; i < 256; ++i) close(i);
+  #endif
+
+    /* reset_signals which may have been ignored (SIG_IGN) */
+  #ifdef SIGTTOU
+    signal(SIGTTOU, SIG_DFL);
+  #endif
+  #ifdef SIGTTIN
+    signal(SIGTTIN, SIG_DFL);
+  #endif
+  #ifdef SIGTSTP
+    signal(SIGTSTP, SIG_DFL);
+  #endif
+    signal(SIGPIPE, SIG_DFL);
+
+    execve(name, argv, envp ? envp : environ);
+
+    if (0 == memcmp(argv[0], "/bin/sh", sizeof("/bin/sh")-1)
+        && argv[1] && 0 == memcmp(argv[1], "-c", sizeof("-c")-1))
+        perror(argv[2]);
+    else
+        perror(argv[0]);
+    _exit(errno);
+
+ #else
+
+    UNUSED(name);
+    UNUSED(argv);
+    UNUSED(envp);
+    UNUSED(fdin);
+    UNUSED(fdout);
+    UNUSED(fderr);
+    UNUSED(dfd);
+    return (pid_t)-1;
+
+ #endif
+}
+
+
+static int fdevent_open_logger_pipe(const char *logger) {
+    /* create write pipe and spawn process */
+    char *args[4];
+    int to_log_fds[2];
+    int devnull = fdevent_open_devnull();
+    pid_t pid;
+
+    if (-1 == devnull) {
+        return -1;
+    }
+
+    if (pipe(to_log_fds)) {
+        close(devnull);
+        return -1;
+    }
+    fdevent_setfd_cloexec(to_log_fds[0]);
+    fdevent_setfd_cloexec(to_log_fds[1]);
+
+    *(const char **)&args[0] = "/bin/sh";
+    *(const char **)&args[1] = "-c";
+    *(const char **)&args[2] = logger;
+    args[3] = NULL;
+
+    pid = fdevent_fork_execve(args[0], args, NULL, to_log_fds[0], devnull, devnull, -1);
+
+    if (pid > 0) {
+        /*(future: save pipe fds, pid, and command line to restart if child exits)*/
+        close(devnull);
+        close(to_log_fds[0]);
+        return to_log_fds[1];
+    }
+    else {
+        int errnum = errno;
+        close(devnull);
+        close(to_log_fds[0]);
+        close(to_log_fds[1]);
+        errno = errnum;
+        return -1;
+    }
+}
+
+
+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+
+int fdevent_open_logger(const char *logger) {
+    if (logger[0] != '|') {
+        int flags = O_APPEND | O_WRONLY | O_CREAT | O_LARGEFILE;
+        return fdevent_open_cloexec(logger, flags, 0644);
+    }
+    else {
+        return fdevent_open_logger_pipe(logger+1); /*(skip the '|')*/
+    }
+}
+
+
 #include <sys/ioctl.h>
 #ifdef HAVE_SYS_FILIO_H
 #include <sys/filio.h>  /* FIONREAD (for illumos (OpenIndiana)) */

src/fdevent.h

@@ -220,6 +220,12 @@ int fdevent_open_cloexec(const char *pathname, int flags, mode_t mode);
 struct sockaddr;
 int fdevent_accept_listenfd(int listenfd, struct sockaddr *addr, size_t *addrlen);
 
+int fdevent_open_devnull(void);
+int fdevent_open_dirname(char *path);
+int fdevent_set_stdin_stdout_stderr(int fdin, int fdout, int fderr);
+pid_t fdevent_fork_execve(const char *name, char *argv[], char *envp[], int fdin, int fdout, int fderr, int dfd);
+int fdevent_open_logger(const char *logger);
+
 int fdevent_select_init(fdevents *ev);
 int fdevent_poll_init(fdevents *ev);
 int fdevent_linux_sysepoll_init(fdevents *ev);

src/log.c

@@ -3,32 +3,18 @@
 #include "base.h"
 #include "fdevent.h"
 #include "log.h"
-#include "array.h"
 
 #include <sys/types.h>
-
 #include <errno.h>
-#include <fcntl.h>
 #include <time.h>
 #include <unistd.h>
 #include <string.h>
-#include <stdlib.h>
-
 #include <stdarg.h>
-#include <stdio.h>
 
 #ifdef HAVE_SYSLOG_H
 # include <syslog.h>
 #endif
 
-#ifdef HAVE_VALGRIND_VALGRIND_H
-# include <valgrind/valgrind.h>
-#endif
-
-#ifndef O_LARGEFILE
-# define O_LARGEFILE 0
-#endif
-
 #ifndef HAVE_CLOCK_GETTIME
 #ifdef HAVE_SYS_TIME_H
 # include <sys/time.h>  /* gettimeofday() */
@@ -79,109 +65,6 @@ ssize_t write_all(int fd, const void* buf, size_t count) {
 	return written;
 }
 
-/* Close fd and _try_ to get a /dev/null for it instead.
- * close() alone may trigger some bugs when a
- * process opens another file and gets fd = STDOUT_FILENO or STDERR_FILENO
- * and later tries to just print on stdout/stderr
- *
- * Returns 0 on success and -1 on failure (fd gets closed in all cases)
- */
-int openDevNull(int fd) {
-	int tmpfd;
-	close(fd);
-#if defined(__WIN32)
-	/* Cygwin should work with /dev/null */
-	tmpfd = open("nul", O_RDWR);
-#else
-	tmpfd = open("/dev/null", O_RDWR);
-#endif
-	if (tmpfd != -1 && tmpfd != fd) {
-		dup2(tmpfd, fd);
-		close(tmpfd);
-	}
-	/* coverity[leaked_handle : FALSE] */
-	return (tmpfd != -1) ? 0 : -1;
-}
-
-int open_logfile_or_pipe(server *srv, const char* logfile) {
-	int fd;
-
-	if (logfile[0] == '|') {
-#ifdef HAVE_FORK
-		/* create write pipe and spawn process */
-
-		int to_log_fds[2];
-
-		if (pipe(to_log_fds)) {
-			log_error_write(srv, __FILE__, __LINE__, "ss", "pipe failed: ", strerror(errno));
-			return -1;
-		}
-
-		/* fork, execve */
-		switch (fork()) {
-		case 0:
-			/* child */
-			close(STDIN_FILENO);
-
-			/* dup the filehandle to STDIN */
-			if (to_log_fds[0] != STDIN_FILENO) {
-				if (STDIN_FILENO != dup2(to_log_fds[0], STDIN_FILENO)) {
-					log_error_write(srv, __FILE__, __LINE__, "ss",
-						"dup2 failed: ", strerror(errno));
-					exit(-1);
-				}
-				close(to_log_fds[0]);
-			}
-			close(to_log_fds[1]);
-
-#ifndef FD_CLOEXEC
-			{
-				int i;
-				/* we don't need the client socket */
-				for (i = 3; i < 256; i++) {
-					close(i);
-				}
-			}
-#endif
-
-			/* close old stderr */
-			openDevNull(STDERR_FILENO);
-
-			/* exec the log-process (skip the | ) */
-			execl("/bin/sh", "sh", "-c", logfile + 1, NULL);
-			log_error_write(srv, __FILE__, __LINE__, "sss",
-					"spawning log process failed: ", strerror(errno),
-					logfile + 1);
-
-			exit(-1);
-			break;
-		case -1:
-			/* error */
-			log_error_write(srv, __FILE__, __LINE__, "ss", "fork failed: ", strerror(errno));
-			return -1;
-		default:
-			close(to_log_fds[0]);
-			fd = to_log_fds[1];
-			break;
-		}
-
-#else
-		return -1;
-#endif
-	} else if (-1 == (fd = open(logfile, O_APPEND | O_WRONLY | O_CREAT | O_LARGEFILE, 0644))) {
-		log_error_write(srv, __FILE__, __LINE__, "SSSS",
-				"opening errorlog '", logfile,
-				"' failed: ", strerror(errno));
-
-		return -1;
-	}
-
-	fd_close_on_exec(fd);
-
-	return fd;
-}
-
-
 /**
  * open the errorlog
  *
@@ -196,6 +79,7 @@ int open_logfile_or_pipe(server *srv, const char* logfile) {
  */
 
 int log_error_open(server *srv) {
+	int errfd;
 #ifdef HAVE_SYSLOG_H
 	/* perhaps someone wants to use syslog() */
 	int facility = -1;
@@ -272,7 +156,10 @@ int log_error_open(server *srv) {
 	} else if (!buffer_string_is_empty(srv->srvconf.errorlog_file)) {
 		const char *logfile = srv->srvconf.errorlog_file->ptr;
 
-		if (-1 == (srv->errorlog_fd = open_logfile_or_pipe(srv, logfile))) {
+		if (-1 == (srv->errorlog_fd = fdevent_open_logger(logfile))) {
+			log_error_write(srv, __FILE__, __LINE__, "SSSS",
+					"opening errorlog '", logfile,
+					"' failed: ", strerror(errno));
 			return -1;
 		}
 		srv->errorlog_mode = (logfile[0] == '|') ? ERRORLOG_PIPE : ERRORLOG_FILE;
@@ -286,7 +173,6 @@ int log_error_open(server *srv) {
 	}
 
 	if (!buffer_string_is_empty(srv->srvconf.breakagelog_file)) {
-		int breakage_fd;
 		const char *logfile = srv->srvconf.breakagelog_file->ptr;
 
 		if (srv->errorlog_mode == ERRORLOG_FD) {
@@ -294,18 +180,32 @@ int log_error_open(server *srv) {
 			fd_close_on_exec(srv->errorlog_fd);
 		}
 
-		if (-1 == (breakage_fd = open_logfile_or_pipe(srv, logfile))) {
+		if (-1 == (errfd = fdevent_open_logger(logfile))) {
+			log_error_write(srv, __FILE__, __LINE__, "SSSS",
+					"opening errorlog '", logfile,
+					"' failed: ", strerror(errno));
 			return -1;
 		}
-
-		if (STDERR_FILENO != breakage_fd) {
-			dup2(breakage_fd, STDERR_FILENO);
-			close(breakage_fd);
-		}
 	} else if (!srv->srvconf.dont_daemonize) {
-		/* move stderr to /dev/null */
-		openDevNull(STDERR_FILENO);
+		/* move STDERR_FILENO to /dev/null */
+		if (-1 == (errfd = fdevent_open_devnull())) {
+			log_error_write(srv, __FILE__, __LINE__, "ss",
+					"opening /dev/null failed:", strerror(errno));
+			return -1;
+		}
+	} else {
+		/*(leave STDERR_FILENO as-is)*/
+		errfd = -1;
+	}
+
+	if (0 != fdevent_set_stdin_stdout_stderr(-1, -1, errfd)) {
+		log_error_write(srv, __FILE__, __LINE__, "ss",
+				"setting stderr failed:", strerror(errno));
+		close(errfd);
+		return -1;
 	}
+
+	if (-1 != errfd) close(errfd);
 	return 0;
 }
 
@@ -326,7 +226,7 @@ int log_error_cycle(server *srv) {
 
 		int new_fd;
 
-		if (-1 == (new_fd = open_logfile_or_pipe(srv, logfile))) {
+		if (-1 == (new_fd = fdevent_open_logger(logfile))) {
 			/* write to old log */
 			log_error_write(srv, __FILE__, __LINE__, "SSSSS",
 					"cycling errorlog '", logfile,

src/log.h

@@ -9,13 +9,6 @@ int log_clock_gettime_realtime (struct timespec *ts);
 
 ssize_t write_all(int fd, const void* buf, size_t count);
 
-/* Close fd and _try_ to get a /dev/null for it instead.
- * Returns 0 on success and -1 on failure (fd gets closed in all cases)
- */
-int openDevNull(int fd);
-
-int open_logfile_or_pipe(server *srv, const char* logfile);
-
 int log_error_open(server *srv);
 int log_error_close(server *srv);
 int log_error_write(server *srv, const char *filename, unsigned int line, const char *fmt, ...);

src/mod_accesslog.c

@@ -630,14 +630,21 @@ SETDEFAULTS_FUNC(log_access_open) {
 
 		if (srv->srvconf.preflight_check) continue;
 
-		if (-1 == (s->log_access_fd = open_logfile_or_pipe(srv, s->access_logfile->ptr)))
+		if (-1 == (s->log_access_fd = fdevent_open_logger(s->access_logfile->ptr))) {
+			log_error_write(srv, __FILE__, __LINE__, "SBSS",
+					"opening log '", s->access_logfile,
+					"' failed: ", strerror(errno));
 			return HANDLER_ERROR;
-
+		}
 	}
 
 	return HANDLER_GO_ON;
 }
 
+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+
 SIGHUP_FUNC(log_access_cycle) {
 	plugin_data *p = p_d;
 	size_t i;

src/mod_cgi.c

@@ -21,9 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <fdevent.h>
-#include <signal.h>
 
-#include <stdio.h>
 #include <fcntl.h>
 
 static int pipe_cloexec(int pipefd[2]) {
@@ -284,8 +282,6 @@ static void cgi_connection_close(server *srv, handler_ctx *hctx) {
 	plugin_data *p = hctx->plugin_data;
 	connection *con = hctx->remote_conn;
 
-#ifndef __WIN32
-
 	/* the connection to the browser went away, but we still have a connection
 	 * to the CGI script
 	 *
@@ -310,7 +306,7 @@ static void cgi_connection_close(server *srv, handler_ctx *hctx) {
 	cgi_handler_ctx_free(hctx);
 
 	/* if waitpid hasn't been called by response.c yet, do it here */
-	if (pid) {
+	if (pid > 0) {
 		/* check if the CGI-script is already gone */
 		switch(waitpid(pid, &status, WNOHANG)) {
 		case 0:
@@ -359,7 +355,6 @@ static void cgi_connection_close(server *srv, handler_ctx *hctx) {
 			cgi_pid_add(srv, p, pid);
 		}
 	}
-#endif
 
 	/* finish response (if not already con->file_started, con->file_finished) */
 	if (con->mode == p->id) {
@@ -771,17 +766,16 @@ static int cgi_write_request(server *srv, handler_ctx *hctx, int fd) {
 }
 
 static int cgi_create_env(server *srv, connection *con, plugin_data *p, handler_ctx *hctx, buffer *cgi_handler) {
-	pid_t pid;
-
+	char_array env;
+	char *args[3];
 	int to_cgi_fds[2];
 	int from_cgi_fds[2];
-	struct stat st;
+	int dfd = -1;
 	UNUSED(p);
 
-#ifndef __WIN32
-
 	if (!buffer_string_is_empty(cgi_handler)) {
 		/* stat the exec file */
+		struct stat st;
 		if (-1 == (stat(cgi_handler->ptr, &st))) {
 			log_error_write(srv, __FILE__, __LINE__, "sbss",
 					"stat for cgi-handler", cgi_handler,
@@ -794,42 +788,20 @@ static int cgi_create_env(server *srv, connection *con, plugin_data *p, handler_
 		log_error_write(srv, __FILE__, __LINE__, "ss", "pipe failed:", strerror(errno));
 		return -1;
 	}
-
 	if (pipe_cloexec(from_cgi_fds)) {
 		close(to_cgi_fds[0]);
 		close(to_cgi_fds[1]);
 		log_error_write(srv, __FILE__, __LINE__, "ss", "pipe failed:", strerror(errno));
 		return -1;
 	}
+	fdevent_setfd_cloexec(to_cgi_fds[1]);
+	fdevent_setfd_cloexec(from_cgi_fds[0]);
 
-	/* fork, execve */
-	switch (pid = fork()) {
-	case 0: {
-		/* child */
-		char **args;
-		int argc;
+	{
 		int i = 0;
-		char_array env;
-		char *c;
 		const char *s;
 		http_cgi_opts opts = { 0, 0, NULL, NULL };
 
-		/* move stdout to from_cgi_fd[1] */
-		dup2(from_cgi_fds[1], STDOUT_FILENO);
-	      #ifndef FD_CLOEXEC
-		close(from_cgi_fds[1]);
-		/* not needed */
-		close(from_cgi_fds[0]);
-	      #endif
-
-		/* move the stdin to to_cgi_fd[0] */
-		dup2(to_cgi_fds[0], STDIN_FILENO);
-	      #ifndef FD_CLOEXEC
-		close(to_cgi_fds[0]);
-		/* not needed */
-		close(to_cgi_fds[1]);
-	      #endif
-
 		/* create environment */
 		env.ptr = NULL;
 		env.size = 0;
@@ -860,9 +832,6 @@ static int cgi_create_env(server *srv, connection *con, plugin_data *p, handler_
 		env.ptr[env.used] = NULL;
 
 		/* set up args */
-		argc = 3;
-		args = malloc(sizeof(*args) * argc);
-		force_assert(args);
 		i = 0;
 
 		if (!buffer_string_is_empty(cgi_handler)) {
@@ -870,51 +839,34 @@ static int cgi_create_env(server *srv, connection *con, plugin_data *p, handler_
 		}
 		args[i++] = con->physical.path->ptr;
 		args[i  ] = NULL;
+	}
 
-		/* search for the last / */
-		if (NULL != (c = strrchr(con->physical.path->ptr, '/'))) {
-			/* handle special case of file in root directory */
-			const char* physdir = (c == con->physical.path->ptr) ? "/" : con->physical.path->ptr;
-
-			/* temporarily shorten con->physical.path to directory without terminating '/' */
-			*c = '\0';
-			/* change to the physical directory */
-			if (-1 == chdir(physdir)) {
-				log_error_write(srv, __FILE__, __LINE__, "ssb", "chdir failed:", strerror(errno), con->physical.path);
-			}
-			*c = '/';
-		}
+	dfd = fdevent_open_dirname(con->physical.path->ptr);
+	if (-1 == dfd) {
+		log_error_write(srv, __FILE__, __LINE__, "ssb", "open dirname failed:", strerror(errno), con->physical.path);
+	}
 
-		/* we don't need the client socket */
-		for (i = 3; i < 256; i++) {
-			if (i != srv->errorlog_fd) close(i);
-		}
+	hctx->pid = (dfd >= 0) ? fdevent_fork_execve(con->physical.path->ptr, args, env.ptr, to_cgi_fds[0], from_cgi_fds[1], -1, dfd) : -1;
 
-		/* exec the cgi */
-		execve(args[0], args, env.ptr);
+	for (size_t i = 0; i < env.used; ++i) free(env.ptr[i]);
+	free(env.ptr);
 
-		/* most log files may have been closed/redirected by this point,
-		 * though stderr might still point to lighttpd.breakage.log */
-		perror(args[0]);
-		_exit(1);
-	}
-	case -1:
-		/* error */
+	if (-1 == hctx->pid) {
+		/* log error with errno prior to calling close() (might change errno) */
 		log_error_write(srv, __FILE__, __LINE__, "ss", "fork failed:", strerror(errno));
+		if (-1 != dfd) close(dfd);
 		close(from_cgi_fds[0]);
 		close(from_cgi_fds[1]);
 		close(to_cgi_fds[0]);
 		close(to_cgi_fds[1]);
 		return -1;
-	default: {
-		/* parent process */
-
+	} else {
+		if (-1 != dfd) close(dfd);
 		close(from_cgi_fds[1]);
 		close(to_cgi_fds[0]);
 
 		/* register PID and wait for them asynchronously */
 
-		hctx->pid = pid;
 		hctx->fd = from_cgi_fds[0];
 		hctx->fde_ndx = -1;
 
@@ -948,14 +900,8 @@ static int cgi_create_env(server *srv, connection *con, plugin_data *p, handler_
 		}
 		fdevent_event_set(srv->ev, &(hctx->fde_ndx), hctx->fd, FDEVENT_IN);
 
-		break;
-	}
+		return 0;
 	}
-
-	return 0;
-#else
-	return -1;
-#endif
 }
 
 static buffer * cgi_get_handler(array *a, buffer *fn) {
@@ -1073,7 +1019,6 @@ TRIGGER_FUNC(cgi_trigger) {
 	plugin_data *p = p_d;
 	size_t ndx;
 	/* the trigger handle only cares about lonely PID which we have to wait for */
-#ifndef __WIN32
 
 	for (ndx = 0; ndx < p->cgi_pid.used; ndx++) {
 		int status;
@@ -1122,7 +1067,7 @@ TRIGGER_FUNC(cgi_trigger) {
 			ndx--;
 		}
 	}
-#endif
+
 	return HANDLER_GO_ON;
 }
 

src/mod_fastcgi.c

@@ -348,23 +348,6 @@ typedef struct {
 /* ok, we need a prototype */
 static handler_t fcgi_handle_fdevent(server *srv, void *ctx, int revents);
 
-#ifdef HAVE_FORK
-static void reset_signals(void) {
-#ifdef SIGTTOU
-	signal(SIGTTOU, SIG_DFL);
-#endif
-#ifdef SIGTTIN
-	signal(SIGTTIN, SIG_DFL);
-#endif
-#ifdef SIGTSTP
-	signal(SIGTSTP, SIG_DFL);
-#endif
-	signal(SIGHUP, SIG_DFL);
-	signal(SIGPIPE, SIG_DFL);
-	signal(SIGUSR1, SIG_DFL);
-}
-#endif /* HAVE_FORK */
-
 static void fastcgi_status_copy_procname(buffer *b, fcgi_extension_host *host, fcgi_proc *proc) {
 	buffer_copy_string_len(b, CONST_STR_LEN("fastcgi.backend."));
 	buffer_append_string_buffer(b, host->id);
@@ -788,7 +771,7 @@ FREE_FUNC(mod_fastcgi_free) {
 					host = ex->hosts[n];
 
 					for (proc = host->first; proc; proc = proc->next) {
-						if (proc->pid != 0) {
+						if (proc->pid > 0) {
 							kill(proc->pid, host->kill_signal);
 						}
 
@@ -799,7 +782,7 @@ FREE_FUNC(mod_fastcgi_free) {
 					}
 
 					for (proc = host->unused_procs; proc; proc = proc->next) {
-						if (proc->pid != 0) {
+						if (proc->pid > 0) {
 							kill(proc->pid, host->kill_signal);
 						}
 						if (proc->is_local &&
@@ -826,7 +809,6 @@ FREE_FUNC(mod_fastcgi_free) {
 	return HANDLER_GO_ON;
 }
 
-#ifdef HAVE_FORK
 static int env_add(char_array *env, const char *key, size_t key_len, const char *val, size_t val_len) {
 	char *dst;
 	size_t i;
@@ -841,8 +823,7 @@ static int env_add(char_array *env, const char *key, size_t key_len, const char
 
 	for (i = 0; i < env->used; i++) {
 		if (0 == strncmp(dst, env->ptr[i], key_len + 1)) {
-			/* don't care about free as we are in a forked child which is going to exec(...) */
-			/* free(env->ptr[i]); */
+			free(env->ptr[i]);
 			env->ptr[i] = dst;
 			return 0;
 		}
@@ -916,21 +897,6 @@ static int parse_binpath(char_array *env, buffer *b) {
 
 	return 0;
 }
-#endif /* HAVE_FORK */
-
-#if !defined(HAVE_FORK)
-static int fcgi_spawn_connection(server *srv,
-                                 plugin_data *p,
-                                 fcgi_extension_host *host,
-                                 fcgi_proc *proc) {
-	UNUSED(srv);
-	UNUSED(p);
-	UNUSED(host);
-	UNUSED(proc);
-	return -1;
-}
-
-#else /* -> defined(HAVE_FORK) */
 
 static int fcgi_spawn_connection(server *srv,
                                  plugin_data *p,
@@ -1071,8 +1037,12 @@ static int fcgi_spawn_connection(server *srv,
 
 	if (-1 == status) {
 		/* server is not up, spawn it  */
-		pid_t child;
+		char_array env;
+		char_array arg;
+		buffer *bin_path = NULL;
+		size_t i;
 		int val;
+		int dfd = -1;
 
 		/* reopen socket */
 		if (-1 == (fcgi_fd = fdevent_socket_cloexec(fcgi_addr->sa_family, SOCK_STREAM, 0))) {
@@ -1106,13 +1076,7 @@ static int fcgi_spawn_connection(server *srv,
 			return -1;
 		}
 
-		switch ((child = fork())) {
-		case 0: {
-			size_t i = 0;
-			char *c;
-			char_array env;
-			char_array arg;
-
+		{
 			/* create environment */
 			env.ptr = NULL;
 			env.size = 0;
@@ -1122,19 +1086,6 @@ static int fcgi_spawn_connection(server *srv,
 			arg.size = 0;
 			arg.used = 0;
 
-			if(fcgi_fd != FCGI_LISTENSOCK_FILENO) {
-				dup2(fcgi_fd, FCGI_LISTENSOCK_FILENO);
-				close(fcgi_fd);
-			}
-			else {
-				fdevent_clrfd_cloexec(fcgi_fd);
-			}
-
-			/* we don't need the client socket */
-			for (i = 3; i < 256; i++) {
-				close(i);
-			}
-
 			/* build clean environment */
 			if (host->bin_env_copy->used) {
 				for (i = 0; i < host->bin_env_copy->used; i++) {
@@ -1175,44 +1126,35 @@ static int fcgi_spawn_connection(server *srv,
 
 			env.ptr[env.used] = NULL;
 
-			parse_binpath(&arg, host->bin_path);
-
-			/* chdir into the base of the bin-path,
-			 * search for the last / */
-			if (NULL != (c = strrchr(arg.ptr[0], '/'))) {
-				*c = '\0';
-
-				/* change to the physical directory */
-				if (-1 == chdir(arg.ptr[0])) {
-					*c = '/';
-					log_error_write(srv, __FILE__, __LINE__, "sss", "chdir failed:", strerror(errno), arg.ptr[0]);
-				}
-				*c = '/';
-			}
-
-			reset_signals();
+			bin_path = buffer_init_buffer(host->bin_path);
+			parse_binpath(&arg, bin_path);
+		}
 
-			/* exec the cgi */
-			execve(arg.ptr[0], arg.ptr, env.ptr);
+		dfd = fdevent_open_dirname(arg.ptr[0]);
+		if (-1 == dfd) {
+			log_error_write(srv, __FILE__, __LINE__, "sss", "open dirname failed:", strerror(errno), arg.ptr[0]);
+		}
 
-			/* log_error_write(srv, __FILE__, __LINE__, "sbs",
-					"execve failed for:", host->bin_path, strerror(errno)); */
+		/*(FCGI_LISTENSOCK_FILENO == STDIN_FILENO == 0)*/
+		proc->pid = (dfd >= 0) ? fdevent_fork_execve(arg.ptr[0], arg.ptr, env.ptr, fcgi_fd, -1, -1, dfd) : -1;
 
-			_exit(errno);
+		for (i = 0; i < env.used; ++i) free(env.ptr[i]);
+		free(env.ptr);
+		/*(arg[] contains string references into bin_path)*/
+		/*for (i = 0; i < arg.used; ++i) free(arg.ptr[i]);*/
+		free(arg.ptr);
+		buffer_free(bin_path);
+		if (-1 != dfd) close(dfd);
+		close(fcgi_fd);
 
-			break;
+		if (-1 == proc->pid) {
+			log_error_write(srv, __FILE__, __LINE__, "sb",
+					"fastcgi-backend failed to start:", host->bin_path);
+			return -1;
 		}
-		case -1:
-			/* error */
-			close(fcgi_fd);
-			break;
-		default:
-			/* father */
-			close(fcgi_fd);
 
 			/* register process */
 			proc->is_local = 1;
-			proc->pid = child;
 
 			/* wait */
 			select(0, NULL, NULL, NULL, &tv);
@@ -1226,9 +1168,6 @@ static int fcgi_spawn_connection(server *srv,
 						"If this is PHP, try removing the bytecode caches for now and try again.");
 				return -1;
 			}
-
-			break;
-		}
 	} else {
 		proc->is_local = 0;
 		proc->pid = 0;
@@ -1244,8 +1183,6 @@ static int fcgi_spawn_connection(server *srv,
 	return 0;
 }
 
-#endif /* HAVE_FORK */
-
 static fcgi_extension_host * unixsocket_is_dup(plugin_data *p, size_t used, buffer *unixsocket) {
 	size_t i, j, n;
 	for (i = 0; i < used; ++i) {

src/mod_rrdtool.c

@@ -1,13 +1,12 @@
 #include "first.h"
 
-#include "server.h"
-#include "connections.h"
-#include "response.h"
-#include "connections.h"
+#include "base.h"
+#include "fdevent.h"
 #include "log.h"
 
 #include "plugin.h"
 #include <sys/types.h>
+#include <sys/wait.h>
 
 #include <fcntl.h>
 #include <stdlib.h>
@@ -16,10 +15,6 @@
 #include <errno.h>
 #include <time.h>
 
-#ifdef HAVE_FORK
-/* no need for waitpid if we don't have fork */
-#include <sys/wait.h>
-#endif
 typedef struct {
 	buffer *path_rrdtool_bin;
 	buffer *path_rrd;
@@ -78,13 +73,11 @@ FREE_FUNC(mod_rrd_free) {
 
 	free(p->config_storage);
 
-	if (p->rrdtool_pid) {
+	if (p->rrdtool_pid > 0) {
 		close(p->read_fd);
 		close(p->write_fd);
-#ifdef HAVE_FORK
 		/* collect status */
 		while (-1 == waitpid(p->rrdtool_pid, NULL, 0) && errno == EINTR) ;
-#endif
 	}
 
 	free(p);
@@ -93,9 +86,7 @@ FREE_FUNC(mod_rrd_free) {
 }
 
 static int mod_rrd_create_pipe(server *srv, plugin_data *p) {
-#ifdef HAVE_FORK
-	pid_t pid;
-
+	char *args[3];
 	int to_rrdtool_fds[2];
 	int from_rrdtool_fds[2];
 	if (pipe(to_rrdtool_fds)) {
@@ -103,87 +94,33 @@ static int mod_rrd_create_pipe(server *srv, plugin_data *p) {
 				"pipe failed: ", strerror(errno));
 		return -1;
 	}
-
 	if (pipe(from_rrdtool_fds)) {
 		log_error_write(srv, __FILE__, __LINE__, "ss",
 				"pipe failed: ", strerror(errno));
 		return -1;
 	}
+	fdevent_setfd_cloexec(to_rrdtool_fds[1]);
+	fdevent_setfd_cloexec(from_rrdtool_fds[0]);
+	*(const char **)&args[0] = p->conf.path_rrdtool_bin->ptr;
+	*(const char **)&args[1] = "-";
+	args[2] = NULL;
 
-	/* fork, execve */
-	switch (pid = fork()) {
-	case 0: {
-		/* child */
-		char **args;
-		int argc;
-		int i = 0;
-		char *dash = "-";
-
-		/* move stdout to from_rrdtool_fd[1] */
-		close(STDOUT_FILENO);
-		dup2(from_rrdtool_fds[1], STDOUT_FILENO);
-		close(from_rrdtool_fds[1]);
-		/* not needed */
-		close(from_rrdtool_fds[0]);
-
-		/* move the stdin to to_rrdtool_fd[0] */
-		close(STDIN_FILENO);
-		dup2(to_rrdtool_fds[0], STDIN_FILENO);
-		close(to_rrdtool_fds[0]);
-		/* not needed */
-		close(to_rrdtool_fds[1]);
-
-		/* set up args */
-		argc = 3;
-		args = malloc(sizeof(*args) * argc);
-		i = 0;
-
-		args[i++] = p->conf.path_rrdtool_bin->ptr;
-		args[i++] = dash;
-		args[i  ] = NULL;
-
-		/* we don't need the client socket */
-		for (i = 3; i < 256; i++) {
-			close(i);
-		}
-
-		/* exec the cgi */
-		execv(args[0], args);
-
-		/* log_error_write(srv, __FILE__, __LINE__, "sss", "spawing rrdtool failed: ", strerror(errno), args[0]); */
-
-		/* */
-		SEGFAULT();
-		break;
-	}
-	case -1:
-		/* error */
-		log_error_write(srv, __FILE__, __LINE__, "ss", "fork failed: ", strerror(errno));
-		break;
-	default: {
-		/* father */
+	p->rrdtool_pid = fdevent_fork_execve(args[0], args, NULL, to_rrdtool_fds[0], from_rrdtool_fds[1], -1, -1);
 
+	if (-1 != p->rrdtool_pid) {
 		close(from_rrdtool_fds[1]);
 		close(to_rrdtool_fds[0]);
-
-		/* register PID and wait for them asyncronously */
 		p->write_fd = to_rrdtool_fds[1];
 		p->read_fd = from_rrdtool_fds[0];
-		p->rrdtool_pid = pid;
-
-		fd_close_on_exec(p->write_fd);
-		fd_close_on_exec(p->read_fd);
-
-		break;
-	}
+		return 0;
+	} else {
+		log_error_write(srv, __FILE__, __LINE__, "SBss", "fork/exec(", p->conf.path_rrdtool_bin, "):", strerror(errno));
+		close(to_rrdtool_fds[0]);
+		close(to_rrdtool_fds[1]);
+		close(from_rrdtool_fds[0]);
+		close(from_rrdtool_fds[1]);
+		return -1;
 	}
-
-	return 0;
-#else
-	UNUSED(srv);
-	UNUSED(p);
-	return -1;
-#endif
 }
 
 /* read/write wrappers to catch EINTR */

src/mod_scgi.c

@@ -20,7 +20,6 @@
 #include <limits.h>
 #include <string.h>
 #include <stdlib.h>
-#include <signal.h>
 
 #include "sys-socket.h"
 #include "sys-endian.h"
@@ -327,23 +326,6 @@ static handler_t scgi_handle_fdevent(server *srv, void *ctx, int revents);
 
 int scgi_proclist_sort_down(server *srv, scgi_extension_host *host, scgi_proc *proc);
 
-#ifdef HAVE_FORK
-static void reset_signals(void) {
-#ifdef SIGTTOU
-	signal(SIGTTOU, SIG_DFL);
-#endif
-#ifdef SIGTTIN
-	signal(SIGTTIN, SIG_DFL);
-#endif
-#ifdef SIGTSTP
-	signal(SIGTSTP, SIG_DFL);
-#endif
-	signal(SIGHUP, SIG_DFL);
-	signal(SIGPIPE, SIG_DFL);
-	signal(SIGUSR1, SIG_DFL);
-}
-#endif /* HAVE_FORK */
-
 static handler_ctx * handler_ctx_init(void) {
 	handler_ctx * hctx;
 
@@ -618,7 +600,7 @@ FREE_FUNC(mod_scgi_free) {
 					host = ex->hosts[n];
 
 					for (proc = host->first; proc; proc = proc->next) {
-						if (proc->pid != 0) kill(proc->pid, SIGTERM);
+						if (proc->pid > 0) kill(proc->pid, SIGTERM);
 
 						if (proc->is_local &&
 						    !buffer_string_is_empty(proc->socket)) {
@@ -627,7 +609,7 @@ FREE_FUNC(mod_scgi_free) {
 					}
 
 					for (proc = host->unused_procs; proc; proc = proc->next) {
-						if (proc->pid != 0) kill(proc->pid, SIGTERM);
+						if (proc->pid > 0) kill(proc->pid, SIGTERM);
 
 						if (proc->is_local &&
 						    !buffer_string_is_empty(proc->socket)) {
@@ -649,7 +631,6 @@ FREE_FUNC(mod_scgi_free) {
 	return HANDLER_GO_ON;
 }
 
-#ifdef HAVE_FORK
 static int env_add(char_array *env, const char *key, size_t key_len, const char *val, size_t val_len) {
 	char *dst;
 	size_t i;
@@ -665,8 +646,7 @@ static int env_add(char_array *env, const char *key, size_t key_len, const char
 
 	for (i = 0; i < env->used; i++) {
 		if (0 == strncmp(dst, env->ptr[i], key_len + 1)) {
-			/* don't care about free as we are in a forked child which is going to exec(...) */
-			/* free(env->ptr[i]); */
+			free(env->ptr[i]);
 			env->ptr[i] = dst;
 			return 0;
 		}
@@ -686,21 +666,6 @@ static int env_add(char_array *env, const char *key, size_t key_len, const char
 
 	return 0;
 }
-#endif /* HAVE_FORK */
-
-#if !defined(HAVE_FORK)
-static int scgi_spawn_connection(server *srv,
-                                 plugin_data *p,
-                                 scgi_extension_host *host,
-                                 scgi_proc *proc) {
-	UNUSED(srv);
-	UNUSED(p);
-	UNUSED(host);
-	UNUSED(proc);
-	return -1;
-}
-
-#else /* -> defined(HAVE_FORK) */
 
 static int scgi_spawn_connection(server *srv,
                                  plugin_data *p,
@@ -827,7 +792,10 @@ static int scgi_spawn_connection(server *srv,
 
 	if (-1 == status) {
 		/* server is not up, spawn in  */
-		pid_t child;
+		char_array env;
+		char *args[4];
+		buffer *b;
+		size_t i;
 		int val;
 
 		/* reopen socket */
@@ -863,32 +831,12 @@ static int scgi_spawn_connection(server *srv,
 			return -1;
 		}
 
-		switch ((child = fork())) {
-		case 0: {
-			buffer *b;
-			size_t i = 0;
-			int fd = 0;
-			char_array env;
-
-
+		{
 			/* create environment */
 			env.ptr = NULL;
 			env.size = 0;
 			env.used = 0;
 
-			if (scgi_fd != 0) {
-				dup2(scgi_fd, 0);
-				close(scgi_fd);
-			}
-			else {
-				fdevent_clrfd_cloexec(scgi_fd);
-			}
-
-			/* we don't need the client socket */
-			for (fd = 3; fd < 256; fd++) {
-				close(fd);
-			}
-
 			/* build clean environment */
 			if (host->bin_env_copy->used) {
 				for (i = 0; i < host->bin_env_copy->used; i++) {
@@ -928,35 +876,34 @@ static int scgi_spawn_connection(server *srv,
 			}
 
 			env.ptr[env.used] = NULL;
+		}
 
-			b = buffer_init();
-			buffer_copy_string_len(b, CONST_STR_LEN("exec "));
-			buffer_append_string_buffer(b, host->bin_path);
-
-			reset_signals();
-
-			/* exec the cgi */
-			execle("/bin/sh", "sh", "-c", b->ptr, (char *)NULL, env.ptr);
+		/*(preserve prior behavior for exec of command)*/
+		/*(admin should really prefer to put any complex command into script)*/
+		b = buffer_init();
+		buffer_copy_string_len(b, CONST_STR_LEN("exec "));
+		buffer_append_string_buffer(b, host->bin_path);
+		*(const char **)&args[0] = "/bin/sh";
+		*(const char **)&args[1] = "-c";
+		*(const char **)&args[2] = b->ptr;
+		args[3] = NULL;
 
-			log_error_write(srv, __FILE__, __LINE__, "sbs",
-					"execl failed for:", host->bin_path, strerror(errno));
+		proc->pid = fdevent_fork_execve(args[0], args, env.ptr, scgi_fd, -1, -1, -1);
 
-			_exit(errno);
+		for (i = 0; i < env.used; ++i) free(env.ptr[i]);
+		free(env.ptr);
+		buffer_free(b);
+		close(scgi_fd);
 
-			break;
+		if (-1 == proc->pid) {
+			log_error_write(srv, __FILE__, __LINE__, "sb",
+					"scgi-backend failed to start:", host->bin_path);
+			return -1;
 		}
-		case -1:
-			/* error */
-			close(scgi_fd);
-			break;
-		default:
-			/* father */
-			close(scgi_fd);
 
 			/* register process */
 			proc->last_used = srv->cur_ts;
 			proc->is_local = 1;
-			proc->pid = child;
 
 			/* wait */
 			select(0, NULL, NULL, NULL, &tv);
@@ -966,9 +913,6 @@ static int scgi_spawn_connection(server *srv,
 						"scgi-backend failed to start:", host->bin_path);
 				return -1;
 			}
-
-			break;
-		}
 	} else {
 		proc->is_local = 0;
 		proc->pid = 0;
@@ -984,8 +928,6 @@ static int scgi_spawn_connection(server *srv,
 	return 0;
 }
 
-#endif /* HAVE_FORK */
-
 static scgi_extension_host * unixsocket_is_dup(plugin_data *p, size_t used, buffer *unixsocket) {
 	size_t i, j, n;
 	for (i = 0; i < used; ++i) {
@@ -2604,7 +2546,7 @@ TRIGGER_FUNC(mod_scgi_handle_trigger) {
 				for (proc = host->first; proc; proc = proc->next) {
 					if (proc->load != 0) break;
 					if (host->num_procs <= host->min_procs) break;
-					if (proc->pid == 0) continue;