From 047c0af427ad32dfd7f4bf09135ec3829ff3193e Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Fri, 9 Aug 2024 21:17:52 +0200 Subject: [PATCH] libutil: rename and optimize closeMostFDs this is only used to close non-stdio files in derivation sandboxes. we may as well encode that in its name, drop the unnecessary integer set, and use close_range to deal with the actual closing of files. not only is this clearer, it also makes sandbox setup on linux fast by 1ms each (cherry-picked and adapted from https://git.lix.systems/lix-project/lix/commit/c7d97802e4f59b8621e67cf62275d6a7fde8fe62) Co-authored-by: Eelco Dolstra Co-authored-by: Cole Helbling Co-authored-by: John Ericson --- .../unix/build/local-derivation-goal.cc | 2 +- src/libutil/file-descriptor.hh | 4 +-- src/libutil/unix/file-descriptor.cc | 30 +++++++++++++++---- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index e1035fbddd3..43a9a919121 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -1993,7 +1993,7 @@ void LocalDerivationGoal::runChild() throw SysError("changing into '%1%'", tmpDir); /* Close all other file descriptors. */ - unix::closeMostFDs({STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO}); + unix::closeExtraFDs(); #if __linux__ linux::setPersonality(drv->platform); diff --git a/src/libutil/file-descriptor.hh b/src/libutil/file-descriptor.hh index be61375f64f..bf8354087fa 100644 --- a/src/libutil/file-descriptor.hh +++ b/src/libutil/file-descriptor.hh @@ -143,10 +143,10 @@ public: namespace unix { /** - * Close all file descriptors except those listed in the given set. + * Close all file descriptors except stdio fds (ie 0, 1, 2). * Good practice in child processes. */ -void closeMostFDs(const std::set & exceptions); +void closeExtraFDs(); /** * Set the close-on-exec flag for the given file descriptor. diff --git a/src/libutil/unix/file-descriptor.cc b/src/libutil/unix/file-descriptor.cc index a3af1623fc9..f867199c04e 100644 --- a/src/libutil/unix/file-descriptor.cc +++ b/src/libutil/unix/file-descriptor.cc @@ -120,14 +120,35 @@ void Pipe::create() ////////////////////////////////////////////////////////////////////// -void unix::closeMostFDs(const std::set & exceptions) +#if __linux__ || __FreeBSD__ +// In future we can use a syscall wrapper, but at the moment musl and older glibc version don't support it. +static int unix_close_range(unsigned int first, unsigned int last, int flags) { + return syscall(SYS_close_range, first, last, (unsigned int)flags); +} +#endif + +void unix::closeExtraFDs() +{ + constexpr int MAX_KEPT_FD = 2; + static_assert(std::max({STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO}) == MAX_KEPT_FD); + +#if __linux__ || __FreeBSD__ + // first try to close_range everything we don't care about. if this + // returns an error with these parameters we're running on a kernel + // that does not implement close_range (i.e. pre 5.9) and fall back + // to the old method. we should remove that though, in some future. + if (unix_close_range(MAX_KEPT_FD + 1, ~0U, 0) == 0) { + return; + } +#endif + #if __linux__ try { for (auto & s : std::filesystem::directory_iterator{"/proc/self/fd"}) { checkInterrupt(); auto fd = std::stoi(s.path().filename()); - if (!exceptions.count(fd)) { + if (fd > MAX_KEPT_FD) { debug("closing leaked FD %d", fd); close(fd); } @@ -142,9 +163,8 @@ void unix::closeMostFDs(const std::set & exceptions) #if HAVE_SYSCONF maxFD = sysconf(_SC_OPEN_MAX); #endif - for (int fd = 0; fd < maxFD; ++fd) - if (!exceptions.count(fd)) - close(fd); /* ignore result */ + for (int fd = MAX_KEPT_FD + 1; fd < maxFD; ++fd) + close(fd); /* ignore result */ }