From: Steinar H. Gunderson <sgunderson@bigfoot.com>
Date: Wed, 5 May 2021 17:02:38 +0000 (+0200)
Subject: Set close-on-exec on all file descriptors we open.
X-Git-Tag: 1.5.0~23
X-Git-Url: https://git.sesse.net/?p=cubemap;a=commitdiff_plain;h=6d34c5b6d8e5bec5d1421eadc103f38d206f34f1

Set close-on-exec on all file descriptors we open.

This is useful when we're opening up to fork off child processes,
to avoid various sockets etc. leaking into them (without having to
close all of them explicitly).
---

diff --git a/acceptor.cpp b/acceptor.cpp
index b3cd3c1..8bc16ed 100644
--- a/acceptor.cpp
+++ b/acceptor.cpp
@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <poll.h>
@@ -24,10 +25,10 @@ int create_server_socket(const sockaddr_in6 &addr, SocketType socket_type)
 	// NOTE: We set as non-blocking, so the acceptor thread can notice that we want to shut it down.
 	int server_sock;
 	if (socket_type == TCP_SOCKET) {
-		server_sock = socket(PF_INET6, SOCK_STREAM | SOCK_NONBLOCK, IPPROTO_TCP);
+		server_sock = socket(PF_INET6, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, IPPROTO_TCP);
 	} else {
 		assert(socket_type == UDP_SOCKET);
-		server_sock = socket(PF_INET6, SOCK_DGRAM | SOCK_NONBLOCK, IPPROTO_UDP);
+		server_sock = socket(PF_INET6, SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC, IPPROTO_UDP);
 	}
 	if (server_sock == -1) {
 		log_perror("socket");
@@ -101,10 +102,17 @@ Acceptor::Acceptor(const AcceptorProto &serialized)
 	  certificate_chain(serialized.certificate_chain()),
 	  private_key(serialized.private_key())
 {
+	// Set back the close-on-exec flag for the socket.
+	// (This can't leak into a child, since we haven't been started yet.)
+	fcntl(server_sock, F_SETFD, 1);
 }
 
 AcceptorProto Acceptor::serialize() const
 {
+	// Unset the close-on-exec flag for the socket.
+	// (This can't leak into a child, since there's only one thread left.)
+	fcntl(server_sock, F_SETFD, 0);
+
 	char buf[INET6_ADDRSTRLEN];
 	inet_ntop(addr.sin6_family, &addr.sin6_addr, buf, sizeof(buf));
 
@@ -133,7 +141,7 @@ void Acceptor::do_work()
 		socklen_t addrlen = sizeof(addr);
 
 		// Get a new socket, and set it as nonblocking.
-		int sock = accept4(server_sock, reinterpret_cast<sockaddr *>(&addr), &addrlen, SOCK_NONBLOCK);
+		int sock = accept4(server_sock, reinterpret_cast<sockaddr *>(&addr), &addrlen, SOCK_NONBLOCK | SOCK_CLOEXEC);
 		if (sock == -1 && errno == EINTR) {
 			continue;
 		}
diff --git a/accesslog.cpp b/accesslog.cpp
index 2d5eb05..f10621f 100644
--- a/accesslog.cpp
+++ b/accesslog.cpp
@@ -34,7 +34,7 @@ void AccessLogThread::do_work()
 	if (filename.empty()) {
 		logfp = nullptr;
 	} else {
-		logfp = fopen(filename.c_str(), "a+");
+		logfp = fopen(filename.c_str(), "a+e");
 		if (logfp == nullptr) {
 			log_perror(filename.c_str());
 			// Continue as before.
diff --git a/client.cpp b/client.cpp
index f02050b..26806d6 100644
--- a/client.cpp
+++ b/client.cpp
@@ -1,8 +1,10 @@
 #include <stdio.h>
 #include <arpa/inet.h>
+#include <fcntl.h>
 #include <netinet/in.h>
 #include <stdint.h>
 #include <sys/socket.h>
+#include <unistd.h>
 
 #include "client.h"
 #include "log.h"
@@ -68,6 +70,10 @@ Client::Client(const ClientProto &serialized, const vector<shared_ptr<const stri
 	  bytes_lost(serialized.bytes_lost()),
 	  num_loss_events(serialized.num_loss_events())
 {
+	// Set back the close-on-exec flag for the socket.
+	// (This can't leak into a child, since we haven't been started yet.)
+	fcntl(sock, F_SETFD, 1);
+
 	if (stream != nullptr) {
 		if (setsockopt(sock, SOL_SOCKET, SO_MAX_PACING_RATE, &stream->pacing_rate, sizeof(stream->pacing_rate)) == -1) {
 			if (stream->pacing_rate != ~0U) {
@@ -115,6 +121,10 @@ Client::Client(const ClientProto &serialized, const vector<shared_ptr<const stri
 
 ClientProto Client::serialize(unordered_map<const string *, size_t> *short_response_pool) const
 {
+	// Unset the close-on-exec flag for the socket.
+	// (This can't leak into a child, since there's only one thread left.)
+	fcntl(sock, F_SETFD, 0);
+
 	ClientProto serialized;
 	serialized.set_sock(sock);
 	serialized.set_remote_addr(remote_addr);
diff --git a/httpinput.cpp b/httpinput.cpp
index 9cacc1f..d0e1c5c 100644
--- a/httpinput.cpp
+++ b/httpinput.cpp
@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <math.h>
 #include <netdb.h>
 #include <netinet/in.h>
@@ -73,6 +74,10 @@ HTTPInput::HTTPInput(const InputProto &serialized)
 	  has_metacube_header(serialized.has_metacube_header()),
 	  sock(serialized.sock())
 {
+	// Set back the close-on-exec flag for the socket.
+	// (This can't leak into a child, since we haven't been started yet.)
+	fcntl(sock, F_SETFD, 1);
+
 	pending_data.resize(serialized.pending_data().size());
 	memcpy(&pending_data[0], serialized.pending_data().data(), serialized.pending_data().size());
 
@@ -111,6 +116,10 @@ void HTTPInput::close_socket()
 
 InputProto HTTPInput::serialize() const
 {
+	// Unset the close-on-exec flag for the socket.
+	// (This can't leak into a child, since there's only one thread left.)
+	fcntl(sock, F_SETFD, 0);
+
 	InputProto serialized;
 	serialized.set_state(state);
 	serialized.set_url(url);
@@ -155,7 +164,7 @@ int HTTPInput::lookup_and_connect(const string &host, const string &port)
 	for ( ; ai && !should_stop(); ai = ai->ai_next) {
 		// Now do a non-blocking connect. This is important because we want to be able to be
 		// woken up, even though it's rather cumbersome.
-		int sock = socket(ai->ai_family, SOCK_STREAM | SOCK_NONBLOCK, IPPROTO_TCP);
+		int sock = socket(ai->ai_family, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, IPPROTO_TCP);
 		if (sock == -1) {
 			// Could be e.g. EPROTONOSUPPORT. The show must go on.
 			continue;
diff --git a/input_stats.cpp b/input_stats.cpp
index f909113..eee597b 100644
--- a/input_stats.cpp
+++ b/input_stats.cpp
@@ -31,7 +31,7 @@ void InputStatsThread::do_work()
 
 		// Open a new, temporary file.
 		char *filename = strdup((stats_file + ".new.XXXXXX").c_str());
-		fd = mkostemp(filename, O_WRONLY);
+		fd = mkostemp(filename, O_WRONLY | O_CLOEXEC);
 		if (fd == -1) {
 			log_perror(filename);
 			free(filename);
diff --git a/log.cpp b/log.cpp
index 198ceae..cbecba0 100644
--- a/log.cpp
+++ b/log.cpp
@@ -1,11 +1,13 @@
 #include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <stdarg.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <string.h>
 #include <syslog.h>
 #include <time.h>
+#include <unistd.h>
 #include <string>
 #include <vector>
 
@@ -23,7 +25,7 @@ vector<FILE *> log_destinations;
 
 void add_log_destination_file(const string &filename)
 {
-	FILE *fp = fopen(filename.c_str(), "a");
+	FILE *fp = fopen(filename.c_str(), "ae");
 	if (fp == nullptr) {
 		perror(filename.c_str());
 		return;
diff --git a/main.cpp b/main.cpp
index 737f0c3..18976fc 100644
--- a/main.cpp
+++ b/main.cpp
@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <getopt.h>
 #include <limits.h>
 #include <signal.h>
@@ -661,6 +662,10 @@ start:
 	char buf[16];
 	sprintf(buf, "%d", state_fd);
 
+	// Unset the close-on-exec flag for the state fd.
+	// (This can't leak into a child, since there's only one thread left.)
+	fcntl(state_fd, F_SETFD, 0);
+
 	for ( ;; ) {
 		execlp(argv0_canon, argv0_canon, config_filename_canon, "--state", buf, nullptr);
 		open_logs(config.log_destinations);
diff --git a/server.cpp b/server.cpp
index 35cb9d6..a6f4574 100644
--- a/server.cpp
+++ b/server.cpp
@@ -60,7 +60,7 @@ inline bool is_earlier(timespec a, timespec b)
 
 Server::Server()
 {
-	epoll_fd = epoll_create(1024);  // Size argument is ignored.
+	epoll_fd = epoll_create1(EPOLL_CLOEXEC);
 	if (epoll_fd == -1) {
 		log_perror("epoll_fd");
 		exit(1);
diff --git a/stats.cpp b/stats.cpp
index 955f2e9..6685a2e 100644
--- a/stats.cpp
+++ b/stats.cpp
@@ -41,7 +41,7 @@ void StatsThread::do_work()
 
 		// Open a new, temporary file.
 		filename = strdup((stats_file + ".new.XXXXXX").c_str());
-		fd = mkostemp(filename, O_WRONLY);
+		fd = mkostemp(filename, O_WRONLY | O_CLOEXEC);
 		if (fd == -1) {
 			log_perror(filename);
 			free(filename);
diff --git a/udpinput.cpp b/udpinput.cpp
index 4b421a3..ecd099a 100644
--- a/udpinput.cpp
+++ b/udpinput.cpp
@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <poll.h>
 #include <stddef.h>
 #include <stdlib.h>
@@ -120,6 +121,10 @@ UDPInput::UDPInput(const InputProto &serialized)
 	: url(serialized.url()),
 	  sock(serialized.sock())
 {
+	// Set back the close-on-exec flag for the socket.
+	// (This can't leak into a child, since we haven't been started yet.)
+	fcntl(sock, F_SETFD, 1);
+
 	// Should be verified by the caller.
 	string protocol;
 	bool ok = parse_url(url, &protocol, &user, &host, &port, &path);
@@ -139,6 +144,10 @@ UDPInput::UDPInput(const InputProto &serialized)
 
 InputProto UDPInput::serialize() const
 {
+	// Unset the close-on-exec flag for the socket.
+	// (This can't leak into a child, since there's only one thread left.)
+	fcntl(sock, F_SETFD, 0);
+
 	InputProto serialized;
 	serialized.set_url(url);
 	serialized.set_sock(sock);
diff --git a/udpstream.cpp b/udpstream.cpp
index 7747649..bbbcebb 100644
--- a/udpstream.cpp
+++ b/udpstream.cpp
@@ -13,7 +13,7 @@
 UDPStream::UDPStream(const sockaddr_in6 &dst, uint32_t pacing_rate, int ttl, int multicast_iface_index)
 	: dst(dst)
 {
-	sock = socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP);
+	sock = socket(AF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, IPPROTO_UDP);
 	if (sock == -1) {
 		// Oops. Ignore this output, then.
 		log_perror("socket");
diff --git a/util.cpp b/util.cpp
index a85de44..10e469d 100644
--- a/util.cpp
+++ b/util.cpp
@@ -18,7 +18,7 @@ using namespace std;
 
 int make_tempfile(const string &contents)
 {
-	int fd = open("/tmp", O_RDWR | O_TMPFILE, 0600);
+	int fd = open("/tmp", O_RDWR | O_TMPFILE | O_CLOEXEC, 0600);
 	if (fd == -1) {
 		char filename[] = "/tmp/cubemap.XXXXXX";
 		mode_t old_umask = umask(077);