Set close-on-exec on all file descriptors we open.
authorSteinar H. Gunderson <sgunderson@bigfoot.com>
Wed, 5 May 2021 17:02:38 +0000 (19:02 +0200)
committerSteinar H. Gunderson <sgunderson@bigfoot.com>
Wed, 5 May 2021 18:16:24 +0000 (20:16 +0200)
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).

12 files changed:
acceptor.cpp
accesslog.cpp
client.cpp
httpinput.cpp
input_stats.cpp
log.cpp
main.cpp
server.cpp
stats.cpp
udpinput.cpp
udpstream.cpp
util.cpp

index b3cd3c1bd92824d27840ef52a04b95bed22e5c62..8bc16ed846d8f23870779682ffd7118486259e2a 100644 (file)
@@ -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;
                }
index 2d5eb0503e9ac00ab2c0bd7207f31eda61f1fa82..f10621f52c5055f17dd01ac0cd7391efa7bc204f 100644 (file)
@@ -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.
index f02050b5588901fd31142558134899f59512464b..26806d6fdcf53d88a2ae758adbbbce7c10e1b214 100644 (file)
@@ -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);
index 9cacc1f0041e8ebccbaccf6c101c35d82a7c899a..d0e1c5c82135f42bf7ac1b75e77d90af5d4a5402 100644 (file)
@@ -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;
index f909113b9024a2642a7acbb5064b6bf34d9bdb92..eee597b273d42cb4b83b2c530bb9913254fe788c 100644 (file)
@@ -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 198ceae9f64633031397fe5117deddb2b013404e..cbecba033824cb84c71bbff2d9bb24c8fcd0c6b1 100644 (file)
--- 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;
index 737f0c3573acea88fa7d9f0585aee69d95ec0838..18976fc833b6d49a981af6fc67e79fe1efa64469 100644 (file)
--- 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);
index 35cb9d6bce869b354f73b96f1824e004cf1835ec..a6f45744e1576b03065c712767bf4cea5078eb45 100644 (file)
@@ -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);
index 955f2e9450ebb6fc7baf73e188fc6eddf372fa62..6685a2e77ea077a0ddb367eb0772859191e2b2b2 100644 (file)
--- 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);
index 4b421a3e4fe90c91732a7a24c53adc476175fd7d..ecd099a529913250b99531f4427fa6c56ec6d91f 100644 (file)
@@ -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);
index 77476499a69a2e13b61d1708ef366f222359424d..bbbcebb008da261d8fea117e31bd7224d5878d21 100644 (file)
@@ -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");
index a85de446a87f9451610f90c401f716e2fab86e5e..10e469df90c0a97c435c1eb405adb6ee5d9b7f7b 100644 (file)
--- 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);