From 9f2be4882d73118d3f4e17471cb64269c7ae94ef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20B=C3=BChler?= <stbuehler@web.de>
Date: Sun, 16 Feb 2014 13:08:29 +0000
Subject: [PATCH] force assertion: setting FD_CLOEXEC must work (if available)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From: Stefan Bühler <stbuehler@web.de>

git-svn-id: svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x@2952 152afb58-edef-0310-8abb-c4023f1b3aa9
---
 NEWS                           |  1 +
 src/connections.c              |  4 +---
 src/fdevent.c                  | 12 +++++++++---
 src/fdevent.h                  |  1 +
 src/fdevent_linux_sysepoll.c   |  9 +--------
 src/fdevent_solaris_devpoll.c  |  9 +--------
 src/log.c                      | 13 +++----------
 src/mod_accesslog.c            |  4 +---
 src/mod_mysql_vhost.c          | 17 +----------------
 src/mod_rrdtool.c              |  6 ++----
 src/mod_trigger_b4_dl.c        |  4 +---
 src/network.c                  |  4 +---
 src/network_freebsd_sendfile.c |  4 +---
 src/network_linux_sendfile.c   |  4 +---
 src/network_writev.c           |  6 +-----
 15 files changed, 26 insertions(+), 72 deletions(-)

diff --git a/NEWS b/NEWS
index e35b7d84..8631fb4b 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,7 @@ NEWS
   * fix resource leaks in error cases on config parsing and other initializations
   * add force_assert() to enforce assertions as simple assert()s are disabled by -DNDEBUG (fixes #2546)
   * [mod_cml_lua] fix null pointer dereference
+  * force assertion: setting FD_CLOEXEC must work (if available)
 
 - 1.4.34
   * [mod_auth] explicitly link ssl for SHA1 (fixes #2517)
diff --git a/src/connections.c b/src/connections.c
index 926b4268..bbaa632f 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -1067,9 +1067,7 @@ found_header_end:
 						if (dst_c->file.fd == -1) {
 							/* this should not happen as we cache the fd, but you never know */
 							dst_c->file.fd = open(dst_c->file.name->ptr, O_WRONLY | O_APPEND);
-#ifdef FD_CLOEXEC
-							fcntl(dst_c->file.fd, F_SETFD, FD_CLOEXEC);
-#endif
+							fd_close_on_exec(dst_c->file.fd);
 						}
 					} else {
 						/* the chunk is too large now, close it */
diff --git a/src/fdevent.c b/src/fdevent.c
index f3582299..3f01c8da 100644
--- a/src/fdevent.c
+++ b/src/fdevent.c
@@ -200,11 +200,17 @@ void * fdevent_get_context(fdevents *ev, int fd) {
 	return ev->fdarray[fd]->ctx;
 }
 
-int fdevent_fcntl_set(fdevents *ev, int fd) {
+void fd_close_on_exec(int fd) {
 #ifdef FD_CLOEXEC
-	/* close fd on exec (cgi) */
-	fcntl(fd, F_SETFD, FD_CLOEXEC);
+	if (fd < 0) return;
+	force_assert(-1 != fcntl(fd, F_SETFD, FD_CLOEXEC));
+#else
+	UNUSED(fd);
 #endif
+}
+
+int fdevent_fcntl_set(fdevents *ev, int fd) {
+	fd_close_on_exec(fd);
 	if ((ev) && (ev->fcntl_set)) return ev->fcntl_set(ev, fd);
 #ifdef O_NONBLOCK
 	return fcntl(fd, F_SETFL, O_NONBLOCK | O_RDWR);
diff --git a/src/fdevent.h b/src/fdevent.h
index 9dd9a6c3..5147baa4 100644
--- a/src/fdevent.h
+++ b/src/fdevent.h
@@ -191,6 +191,7 @@ int fdevent_poll(fdevents *ev, int timeout_ms);
 int fdevent_register(fdevents *ev, int fd, fdevent_handler handler, void *ctx);
 int fdevent_unregister(fdevents *ev, int fd);
 
+void fd_close_on_exec(int fd);
 int fdevent_fcntl_set(fdevents *ev, int fd);
 
 int fdevent_select_init(fdevents *ev);
diff --git a/src/fdevent_linux_sysepoll.c b/src/fdevent_linux_sysepoll.c
index f761ed66..4dec69de 100644
--- a/src/fdevent_linux_sysepoll.c
+++ b/src/fdevent_linux_sysepoll.c
@@ -140,14 +140,7 @@ int fdevent_linux_sysepoll_init(fdevents *ev) {
 		return -1;
 	}
 
-	if (-1 == fcntl(ev->epoll_fd, F_SETFD, FD_CLOEXEC)) {
-		log_error_write(ev->srv, __FILE__, __LINE__, "SSS",
-			"fcntl on epoll-fd failed (", strerror(errno), "), try to set server.event-handler = \"poll\" or \"select\"");
-
-		close(ev->epoll_fd);
-
-		return -1;
-	}
+	fd_close_on_exec(ev->epoll_fd);
 
 	ev->epoll_events = malloc(ev->maxfds * sizeof(*ev->epoll_events));
 
diff --git a/src/fdevent_solaris_devpoll.c b/src/fdevent_solaris_devpoll.c
index dd273e65..d2933a99 100644
--- a/src/fdevent_solaris_devpoll.c
+++ b/src/fdevent_solaris_devpoll.c
@@ -121,14 +121,7 @@ int fdevent_solaris_devpoll_reset(fdevents *ev) {
 		return -1;
 	}
 
-	if (fcntl(ev->devpoll_fd, F_SETFD, FD_CLOEXEC) < 0) {
-		log_error_write(ev->srv, __FILE__, __LINE__, "SSS",
-			"fcntl /dev/poll fd failed (", strerror(errno), "), try to set server.event-handler = \"poll\" or \"select\"");
-
-		close(ev->devpoll_fd);
-
-		return -1;
-	}
+	fd_close_on_exec(ev->devpoll_fd);
 	return 0;
 }
 int fdevent_solaris_devpoll_init(fdevents *ev) {
diff --git a/src/log.c b/src/log.c
index 4778ee8f..67f6616d 100644
--- a/src/log.c
+++ b/src/log.c
@@ -122,9 +122,7 @@ int open_logfile_or_pipe(server *srv, const char* logfile) {
 		return -1;
 	}
 
-#ifdef FD_CLOEXEC
-	fcntl(fd, F_SETFD, FD_CLOEXEC);
-#endif
+	fd_close_on_exec(fd);
 
 	return fd;
 }
@@ -178,9 +176,7 @@ int log_error_open(server *srv) {
 
 		if (srv->errorlog_mode == ERRORLOG_FD) {
 			srv->errorlog_fd = dup(STDERR_FILENO);
-#ifdef FD_CLOEXEC
-			fcntl(srv->errorlog_fd, F_SETFD, FD_CLOEXEC);
-#endif
+			fd_close_on_exec(srv->errorlog_fd);
 		}
 
 		if (-1 == (breakage_fd = open_logfile_or_pipe(srv, logfile))) {
@@ -231,10 +227,7 @@ int log_error_cycle(server *srv) {
 			/* ok, new log is open, close the old one */
 			close(srv->errorlog_fd);
 			srv->errorlog_fd = new_fd;
-#ifdef FD_CLOEXEC
-			/* close fd on exec (cgi) */
-			fcntl(srv->errorlog_fd, F_SETFD, FD_CLOEXEC);
-#endif
+			fd_close_on_exec(srv->errorlog_fd);
 		}
 	}
 
diff --git a/src/mod_accesslog.c b/src/mod_accesslog.c
index 4bc6cce4..e654fcfc 100644
--- a/src/mod_accesslog.c
+++ b/src/mod_accesslog.c
@@ -612,9 +612,7 @@ SIGHUP_FUNC(log_access_cycle) {
 
 				return HANDLER_ERROR;
 			}
-#ifdef FD_CLOEXEC
-			fcntl(s->log_access_fd, F_SETFD, FD_CLOEXEC);
-#endif
+			fd_close_on_exec(s->log_access_fd);
 		}
 	}
 
diff --git a/src/mod_mysql_vhost.c b/src/mod_mysql_vhost.c
index f9d194bd..538cfff3 100644
--- a/src/mod_mysql_vhost.c
+++ b/src/mod_mysql_vhost.c
@@ -275,22 +275,7 @@ SERVER_FUNC(mod_mysql_vhost_set_defaults) {
 			}
 #undef FOO
 
-#if 0
-			/* set close_on_exec for mysql the hard way */
-			/* Note: this only works as it is done during startup, */
-			/* otherwise we cannot be sure that mysql is fd i-1 */
-			{ int fd;
-			if (-1 != (fd = open("/dev/null", 0))) {
-				close(fd);
-#ifdef FD_CLOEXEC
-				fcntl(fd-1, F_SETFD, FD_CLOEXEC);
-#endif
-			} }
-#else
-#ifdef FD_CLOEXEC
-			fcntl(s->mysql->net.fd, F_SETFD, FD_CLOEXEC);
-#endif
-#endif
+			fd_close_on_exec(s->mysql->net.fd);
 		}
 	}
 
diff --git a/src/mod_rrdtool.c b/src/mod_rrdtool.c
index 65d31c23..df2a7812 100644
--- a/src/mod_rrdtool.c
+++ b/src/mod_rrdtool.c
@@ -170,10 +170,8 @@ static int mod_rrd_create_pipe(server *srv, plugin_data *p) {
 		p->read_fd = from_rrdtool_fds[0];
 		p->rrdtool_pid = pid;
 
-#ifdef FD_CLOEXEC
-		fcntl(p->write_fd, F_SETFD, FD_CLOEXEC);
-		fcntl(p->read_fd, F_SETFD, FD_CLOEXEC);
-#endif
+		fd_close_on_exec(p->write_fd);
+		fd_close_on_exec(p->read_fd);
 
 		break;
 	}
diff --git a/src/mod_trigger_b4_dl.c b/src/mod_trigger_b4_dl.c
index 5eb2d8b2..6d9010d1 100644
--- a/src/mod_trigger_b4_dl.c
+++ b/src/mod_trigger_b4_dl.c
@@ -181,9 +181,7 @@ SETDEFAULTS_FUNC(mod_trigger_b4_dl_set_defaults) {
 						"gdbm-open failed");
 				return HANDLER_ERROR;
 			}
-#ifdef FD_CLOEXEC
-			fcntl(gdbm_fdesc(s->db), F_SETFD, FD_CLOEXEC);
-#endif
+			fd_close_on_exec(gdbm_fdesc(s->db));
 		}
 #endif
 #if defined(HAVE_PCRE_H)
diff --git a/src/network.c b/src/network.c
index 93764848..3c7dcc12 100644
--- a/src/network.c
+++ b/src/network.c
@@ -261,10 +261,8 @@ static int network_server_init(server *srv, buffer *host_token, specific_config
 		}
 	}
 
-#ifdef FD_CLOEXEC
 	/* set FD_CLOEXEC now, fdevent_fcntl_set is called later; needed for pipe-logger forks */
-	fcntl(srv_socket->fd, F_SETFD, FD_CLOEXEC);
-#endif
+	fd_close_on_exec(srv_socket->fd);
 
 	/* */
 	srv->cur_fds = srv_socket->fd;
diff --git a/src/network_freebsd_sendfile.c b/src/network_freebsd_sendfile.c
index 7b165fca..62a024d1 100644
--- a/src/network_freebsd_sendfile.c
+++ b/src/network_freebsd_sendfile.c
@@ -151,9 +151,7 @@ int network_write_chunkqueue_freebsdsendfile(server *srv, connection *con, int f
 					return -1;
 				}
 
-#ifdef FD_CLOEXEC
-				fcntl(c->file.fd, F_SETFD, FD_CLOEXEC);
-#endif
+				fd_close_on_exec(c->file.fd);
 			}
 
 			r = 0;
diff --git a/src/network_linux_sendfile.c b/src/network_linux_sendfile.c
index 91056037..8d7598ac 100644
--- a/src/network_linux_sendfile.c
+++ b/src/network_linux_sendfile.c
@@ -141,9 +141,7 @@ int network_write_chunkqueue_linuxsendfile(server *srv, connection *con, int fd,
 
 					return -1;
 				}
-#ifdef FD_CLOEXEC
-				fcntl(c->file.fd, F_SETFD, FD_CLOEXEC);
-#endif
+				fd_close_on_exec(c->file.fd);
 #ifdef HAVE_POSIX_FADVISE
 				/* tell the kernel that we want to stream the file */
 				if (-1 == posix_fadvise(c->file.fd, 0, 0, POSIX_FADV_SEQUENTIAL)) {
diff --git a/src/network_writev.c b/src/network_writev.c
index e9509306..1b93547f 100644
--- a/src/network_writev.c
+++ b/src/network_writev.c
@@ -230,14 +230,10 @@ int network_write_chunkqueue_writev(server *srv, connection *con, int fd, chunkq
 
 						return -1;
 					}
-#ifdef FD_CLOEXEC
-					fcntl(c->file.fd, F_SETFD, FD_CLOEXEC);
-#endif
+					fd_close_on_exec(c->file.fd);
 				}
 
 				if (MAP_FAILED == (c->file.mmap.start = mmap(NULL, to_mmap, PROT_READ, MAP_SHARED, c->file.fd, c->file.mmap.offset))) {
-					/* close it here, otherwise we'd have to set FD_CLOEXEC */
-
 					log_error_write(srv, __FILE__, __LINE__, "ssbd", "mmap failed:",
 							strerror(errno), c->file.name, c->file.fd);