Merge bitcoin/bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON

a0eed55398f882d9390e50582b10272d18f2b836 run_command: Enable close_fds option to avoid lingering fds (Luke Dashjr)
c7c356a448657e154e6bad6c39a296cfd6dce30c cpp-subprocess: Iterate through /proc/self/fd for close_fds option on Linux (Luke Dashjr)
4f5e04da135080291853f71e6f81dd0302224c3a Revert "remove unneeded close_fds option from cpp-subprocess" (Luke Dashjr)

Pull request description:

  Picks up stale #30756, while addressing my fallback comment (https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2030844440).

  > Currently, RunCommandParseJSON runs its target with whatever fds happen to be open inherited on POSIX platforms. I don't think there's any practical scenario where this is a problem right now, but there's a lot of potential for weird problems (eg, if a process manages to outlive bitcoind - perhaps it's hanging - the listening port(s) won't get released and starting bitcoind again will fail). It's also a potential security issue if a child process is intended to be sandboxed at some point. Not to mention plain ugly :)
  >
  > cpp-subprocess has a feature to address this called close_fds. Not sure why it was removed in https://github.com/bitcoin/bitcoin/pull/29961 rather than fixing this during the migration, but this PR restores it, enables it for RunCommandParseJSON, and optimises it by iterating over /proc/self/fd/ like most other libraries do these days ([eg, glib]> (487b1fd20c/glib/gspawn.c (L1094))) since iterating all possible fd numbers [has been found to be problematic](https://bugzilla.redhat.com/show_bug.cgi?id=1537564).
  >
  > (Equivalent to https://github.com/bitcoin/bitcoin/pull/22417 was for boost::process)

ACKs for top commit:
  achow101:
    ACK a0eed55398f882d9390e50582b10272d18f2b836
  hebasto:
    ACK a0eed55398f882d9390e50582b10272d18f2b836, tested on Ubuntu 25.04:
  vasild:
    ACK a0eed55398f882d9390e50582b10272d18f2b836

Tree-SHA512: 7dc1cb6cc1f45ff7c4f53512e400baad1a033b4ebf14ba6f6ffa38588314932d6d01ef67b197f081e8202bb802659ac6a87998277797721d6d7b20efde8e9a6b

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);
     }

src/util/subprocess.h

@@ -36,6 +36,8 @@ Documentation for C++ subprocessing library.
 #ifndef BITCOIN_UTIL_SUBPROCESS_H
 #define BITCOIN_UTIL_SUBPROCESS_H
 
+#include <util/fs.h>
+#include <util/strencodings.h>
 #include <util/syserror.h>
 
 #include <algorithm>
@@ -536,6 +538,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.
  */
@@ -733,6 +749,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;
@@ -1026,6 +1043,8 @@ private:
   std::future<void> cleanup_future_;
 #endif
 
+  bool close_fds_ = false;
+
   std::string exe_name_;
 
   // Command in string format
@@ -1269,6 +1288,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__
@@ -1315,6 +1338,41 @@ namespace detail {
       if (stream.err_write_ != -1 && stream.err_write_ > 2)
         subprocess_close(stream.err_write_);
 
+      // 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/<pid>/fd.
+        try {
+          std::vector<int> fds_to_close;
+          for (const auto& it : fs::directory_iterator(strprintf("/proc/%d/fd", getpid()))) {
+            auto fd{ToIntegral<uint64_t>(it.path().filename().native())};
+            if (!fd || *fd > std::numeric_limits<int>::max()) continue;
+            if (*fd <= 2) continue;  // leave std{in,out,err} alone
+            if (*fd == static_cast<uint64_t>(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/<pid>/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);
+
+        for (int i = 3; i < max_fd; i++) {
+          if (i == err_wr_pipe_) continue;
+          close(i);
+        }
+#endif
+      }
+
       // Replace the current image with the executable
       sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());