From 4f5e04da135080291853f71e6f81dd0302224c3a Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 29 Aug 2024 19:10:48 +0000 Subject: [PATCH 1/3] Revert "remove unneeded close_fds option from cpp-subprocess" This reverts commit 79c30363733503a1fb7d4c98aa0d56ced0be6e32. --- src/util/subprocess.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/util/subprocess.h b/src/util/subprocess.h index 3449fa3b1ba..545f5879b08 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -520,6 +520,20 @@ namespace util * ------------------------------- */ +/*! + * Option to close all file descriptors + * when the child process is spawned. + * The close fd list does not include + * input/output/error if they are explicitly + * set as part of the Popen arguments. + * + * Default value is false. + */ +struct close_fds { + explicit close_fds(bool c): close_all(c) {} + bool close_all = false; +}; + /*! * Base class for all arguments involving string value. */ @@ -717,6 +731,7 @@ struct ArgumentDeducer void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); + void set_option(close_fds&& cfds); private: Popen* popen_ = nullptr; @@ -1004,6 +1019,8 @@ private: std::future cleanup_future_; #endif + bool close_fds_ = false; + std::string exe_name_; // Command in string format @@ -1233,6 +1250,10 @@ namespace detail { if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; } + inline void ArgumentDeducer::set_option(close_fds&& cfds) { + popen_->close_fds_ = cfds.close_all; + } + inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1279,6 +1300,17 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) close(stream.err_write_); + // Close all the inherited fd's except the error write pipe + if (parent_->close_fds_) { + int max_fd = sysconf(_SC_OPEN_MAX); + if (max_fd == -1) throw OSError("sysconf failed", errno); + + for (int i = 3; i < max_fd; i++) { + if (i == err_wr_pipe_) continue; + close(i); + } + } + // Replace the current image with the executable sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); From c7c356a448657e154e6bad6c39a296cfd6dce30c Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 8 Jul 2021 06:43:04 +0000 Subject: [PATCH 2/3] cpp-subprocess: Iterate through /proc/self/fd for close_fds option on Linux As an optimization, iterate through /proc//fd on Linux. Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com> --- src/util/subprocess.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/util/subprocess.h b/src/util/subprocess.h index 545f5879b08..1a41f966729 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -36,6 +36,8 @@ Documentation for C++ subprocessing library. #ifndef BITCOIN_UTIL_SUBPROCESS_H #define BITCOIN_UTIL_SUBPROCESS_H +#include +#include #include #include @@ -1302,6 +1304,29 @@ namespace detail { // Close all the inherited fd's except the error write pipe if (parent_->close_fds_) { + // If possible, try to get the list of open file descriptors from the + // operating system. This is more efficient, but not guaranteed to be + // available. +#ifdef __linux__ + // For Linux, enumerate /proc//fd. + try { + std::vector fds_to_close; + for (const auto& it : fs::directory_iterator(strprintf("/proc/%d/fd", getpid()))) { + auto fd{ToIntegral(it.path().filename().native())}; + if (!fd || *fd > std::numeric_limits::max()) continue; + if (*fd <= 2) continue; // leave std{in,out,err} alone + if (*fd == static_cast(err_wr_pipe_)) continue; + fds_to_close.push_back(*fd); + } + for (const int fd : fds_to_close) { + close(fd); + } + } catch (const fs::filesystem_error &e) { + throw OSError("/proc//fd iteration failed", e.code().value()); + } +#else + // On other operating systems, iterate over all file descriptor slots + // and try to close them all. int max_fd = sysconf(_SC_OPEN_MAX); if (max_fd == -1) throw OSError("sysconf failed", errno); @@ -1309,6 +1334,7 @@ namespace detail { if (i == err_wr_pipe_) continue; close(i); } +#endif } // Replace the current image with the executable From a0eed55398f882d9390e50582b10272d18f2b836 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 29 Aug 2024 20:05:50 +0000 Subject: [PATCH 3/3] run_command: Enable close_fds option to avoid lingering fds --- src/common/run_command.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index 1f6d51b4f4b..110ec416353 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -24,7 +24,7 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& if (str_command.empty()) return UniValue::VNULL; - auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}); + auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}, sp::close_fds{true}); if (!str_std_in.empty()) { c.send(str_std_in); }