From 261a143acee0aa744c10e1c157a33edf46571918 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Thu, 1 Dec 2022 17:00:30 +0100 Subject: [PATCH 1/3] epoll: avoid fd leaks to child processes by using epoll_create1. There is no use case in which one would need to share libdill's epoll file descriptor with a child process. Even if it was innocuous, it makes automated checking difficult as one has to consider this special case. Since libdill does not provide access to the internal epoll fd, the only way to fix this is to change the source code itself. --- epoll.c.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/epoll.c.inc b/epoll.c.inc index c925125e..6b428b68 100644 --- a/epoll.c.inc +++ b/epoll.c.inc @@ -62,7 +62,7 @@ int dill_ctx_pollset_init(struct dill_ctx_pollset *ctx) { /* Changelist is empty. */ ctx->changelist = DILL_ENDLIST; /* Create the kernel-side pollset. */ - ctx->efd = epoll_create(1); + ctx->efd = epoll_create1(EPOLL_CLOEXEC); if(dill_slow(ctx->efd < 0)) {err = errno; goto error2;} return 0; error2: From 2ed6afbecd05e9ee84b3e54583bb877999b2a5f5 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Thu, 1 Dec 2022 18:11:00 +0100 Subject: [PATCH 2/3] Set close-on-exec on new sockets created through accept(...) This is necessary to avoid leaking file descriptors to child processes. The implementation tries to use accept4 when available, since that sets the flag atomically with the creation of the connected socket, and falls back to accept()+fcntl if not. --- configure.ac | 2 +- fd.c | 23 +++++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index e7a487da..2dc94cc4 100644 --- a/configure.ac +++ b/configure.ac @@ -180,6 +180,7 @@ AC_CHECK_FUNCS([clock_gettime]) AC_CHECK_LIB([socket], [socket]) AC_CHECK_FUNCS([epoll_create], [AC_DEFINE([HAVE_EPOLL])]) AC_CHECK_FUNCS([kqueue], [AC_DEFINE([HAVE_KQUEUE])]) +AC_CHECK_FUNCS([accept4], [AC_DEFINE([HAVE_ACCEPT4])]) dnl Check if struct sockaddr contains sa_len member AC_CHECK_MEMBERS([struct sockaddr.sa_len], [], [], [ @@ -209,4 +210,3 @@ AC_CONFIG_MACRO_DIR([m4]) AC_OUTPUT([Makefile man/Makefile libdill.pc]) cp confdefs.h config.h - diff --git a/fd.c b/fd.c index 92e93887..d3b09eaa 100644 --- a/fd.c +++ b/fd.c @@ -22,6 +22,11 @@ */ +#ifdef HAVE_ACCEPT4 +#define _GNU_SOURCE +#include +#endif + #include #include #include @@ -127,12 +132,27 @@ int dill_fd_connect(int s, const struct sockaddr *addr, socklen_t addrlen, return 0; } +#ifdef HAVE_ACCEPT4 +#define _dill_accept(fd, addr, addrlen) accept4((fd), (addr), (addrlen), SOCK_CLOEXEC) +#else +int _dill_accept(int sockfd, struct sockaddr *addr, socklen_t *addrlen) { + int as = accept(sockfd, addr, addrlen); + if(dill_fast(as >= 0)) { + int fd_flags = fcntl(as, F_GETFD); + if (dill_fast(fd_flags != -1)) { + fcntl(as, F_SETFD, fd_flags | FD_CLOEXEC); + } + } + return as; +} +#endif + int dill_fd_accept(int s, struct sockaddr *addr, socklen_t *addrlen, int64_t deadline) { int as; while(1) { /* Try to accept new connection synchronously. */ - as = accept(s, addr, addrlen); + as = _dill_accept(s, addr, addrlen); if(dill_fast(as >= 0)) break; /* If connection was aborted by the peer grab the next one. */ @@ -426,4 +446,3 @@ int dill_fd_check(int s, int type, int family1, int family2, int listening) { errno = EINVAL; return -1;} return 0; } - From acf0c5095b6dc4c700b68e5ab0ea77b2225c0441 Mon Sep 17 00:00:00 2001 From: Juan Carrano Date: Thu, 1 Dec 2022 19:03:02 +0100 Subject: [PATCH 3/3] dill_fd_own: preserve fd-flags (CLOEXEC) after dup. The IPC and TPC subsystems call dill_fd_own which does not preserve the file descriptor flags. This means that even if the user creates the sockets with the proper flags, when they use "fromfd" the file descriptors will still be leaked to child processes. The implementation tries to use F_DUPFD_CLOEXEC if available. --- fd.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fd.c b/fd.c index d3b09eaa..09130461 100644 --- a/fd.c +++ b/fd.c @@ -419,8 +419,18 @@ void dill_fd_close(int s) { } int dill_fd_own(int s) { +#ifdef F_DUPFD_CLOEXEC + int n = fcntl(s, F_DUPFD_CLOEXEC, 0); +#else + int fd_flags = fcntl(s, F_GETFD); int n = dup(s); +#endif if(dill_slow(n < 0)) return -1; +#ifndef F_DUPFD_CLOEXEC + if (dill_fast(fd_flags != -1)) { + fcntl(n, F_SETFD, fd_flags); + } +#endif dill_fd_close(s); return n; }