[mesos] 05/19: Avoided leaking file descriptors in Mesos containerizer.

[gi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gilbert pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 2e9293a727e78bcff7a0e58fd297aa5f329e8e00
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Mon Jun 25 20:08:48 2018 +0200

    Avoided leaking file descriptors in Mesos containerizer.
    
    This patch explicitly closes not required file descriptors when
    forking a Mesos containerizer instance. We currently only pass on
    stdin, stout, and sterr.
    
    While it would in principle be possible to open all file descriptors
    with `O_CLOEXEC`, this is not practical as
    
    * `O_CLOEXEC` is not supported on BSDs (not Windows either, but file
      descriptor leaks are prevented there in a different way), and
    * we might call thirdparty code outside of our control which does not
      use e.g., `O_CLOEXEC` either (the case e.g., for leveldb).
    
    Closing all file descriptors after the fork avoids leaks even in above
    scenarios.
    
    Review: https://reviews.apache.org/r/67137/
    (cherry picked from commit 3f60c9857f8f68c3d2330ff3d03c8aa84b630db7)
---
 src/slave/containerizer/mesos/launch.cpp | 59 +++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 0affcf5..2749b5d 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -29,8 +29,10 @@
 
 #include <stout/foreach.hpp>
 #include <stout/os.hpp>
-#include <stout/protobuf.hpp>
 #include <stout/path.hpp>
+#include <stout/protobuf.hpp>
+#include <stout/stringify.hpp>
+#include <stout/try.hpp>
 #include <stout/unreachable.hpp>
 
 #include <mesos/mesos.hpp>
@@ -57,6 +59,7 @@
 using std::cerr;
 using std::cout;
 using std::endl;
+using std::list;
 using std::set;
 using std::string;
 using std::vector;
@@ -230,6 +233,41 @@ static void exitWithStatus(int status)
 }
 
 
+// On Windows all new processes create by Mesos go through the
+// `create_process` wrapper which with the completion of MESOS-8926
+// will prevent inadvertent leaks making this code unnecessary there.
+//
+// TODO(bbannier): Consider moving this to stout as e.g., `os::lsof`.
+#ifndef __WINDOWS__
+static Try<hashset<int_fd>> getOpenFileDescriptors()
+{
+  Try<list<string>> fds =
+#if defined(__linux__)
+    os::ls("/proc/self/fd");
+#elif defined(__APPLE__)
+    os::ls("/dev/fd");
+#endif
+
+  if (fds.isError()) {
+    return Error(fds.error());
+  }
+
+  hashset<int_fd> result;
+  foreach (const string& fd, fds.get()) {
+    Try<int_fd> fd_ = numify<int_fd>(fd);
+
+    if (fd_.isError()) {
+      return Error("Could not interpret file descriptor: " + fd_.error());
+    }
+
+    result.insert(fd_.get());
+  }
+
+  return result;
+}
+#endif // __WINDOWS__
+
+
 int MesosContainerizerLaunch::execute()
 {
   if (flags.help) {
@@ -746,6 +784,18 @@ int MesosContainerizerLaunch::execute()
   }
 
 #ifndef __WINDOWS__
+  // Construct a set of file descriptors to close before `exec`'ing.
+  Try<hashset<int_fd>> fds = getOpenFileDescriptors();
+  CHECK_SOME(fds);
+
+  std::set<int_fd> whitelistedFds = {
+    STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO};
+
+  // Exclude required file descriptors from closing.
+  foreach (int_fd fd, whitelistedFds) {
+    fds->erase(fd);
+  }
+
   // If we have `containerStatusFd` set, then we need to fork-exec the
   // command we are launching and checkpoint its status on exit. We
   // use fork-exec directly (as opposed to `process::subprocess()`) to
@@ -816,6 +866,13 @@ int MesosContainerizerLaunch::execute()
       os::close(containerStatusFd.get());
       ::_exit(EXIT_SUCCESS);
     }
+
+    // Avoid leaking not required file descriptors into the forked process.
+    foreach (int_fd fd, fds.get()) {
+      // We use the unwrapped `::close` as opposed to `os::close`
+      // since the former is guaranteed to be async signal safe.
+      ::close(fd);
+    }
   }
 #endif // __WINDOWS__