Updated launch helper to avoid initializing libprocess.

Previously, we used 'process::subprocess()' to run all of our pre-exec
commands. However, doing so causes us to (unnecesssarily) initialize
all of libprocess (and subsequently creating a whole bunch of unused
threads, etc.) just to run a simple script.

To avoid this, we now use `os::system()` and the new `os::spawn()`
functions to give us our shell/non-shell variant of commands we want
to launch.

In the past, we used 'os::system()' alone to avoid initializing
libprocess, but this caused security issues with allowing arbitrary
shell commands to be appended to root-level pre-exec commands that
take strings as their last argument (e.g. mount --bind <src> <target>,
where target is user supplied and is set to "target_dir; rm -rf /").
We now handle this case by using `os::spawn()` instead.

Review: https://reviews.apache.org/r/52011/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bcb33ee1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bcb33ee1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bcb33ee1

Branch: refs/heads/master
Commit: bcb33ee1c8fe51eb0b29d903e5f486edc38204cb
Parents: 4390d2f
Author: Kevin Klues <klue...@gmail.com>
Authored: Thu Sep 22 20:45:55 2016 -0700
Committer: Jie Yu <yujie....@gmail.com>
Committed: Thu Sep 22 20:46:46 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 33 +++++++--------------------
 1 file changed, 8 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/bcb33ee1/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp 
b/src/slave/containerizer/mesos/launch.cpp
index 48ec370..a80c692 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -23,8 +23,6 @@
 #include <iostream>
 #include <set>
 
-#include <process/subprocess.hpp>
-
 #include <stout/foreach.hpp>
 #include <stout/os.hpp>
 #include <stout/protobuf.hpp>
@@ -47,8 +45,6 @@ using std::set;
 using std::string;
 using std::vector;
 
-using process::Subprocess;
-
 #ifdef __linux__
 using mesos::internal::capabilities::Capabilities;
 using mesos::internal::capabilities::Capability;
@@ -232,37 +228,24 @@ int MesosContainerizerLaunch::execute()
 
       cout << "Executing pre-exec command '" << value << "'" << endl;
 
-      Try<Subprocess> s = Error("Not launched");
+      int status = 0;
 
       if (parse->shell()) {
-        s = subprocess(parse->value(), Subprocess::PATH("/dev/null"));
+        // Execute the command using the system shell.
+        status = os::system(parse->value());
       } else {
-        // Launch non-shell command as a subprocess to avoid injecting
-        // arbitrary shell commands.
+        // Directly spawn all non-shell commands to prohibit users
+        // from injecting arbitrary shell commands in the arguments.
         vector<string> args;
         foreach (const string& arg, parse->arguments()) {
           args.push_back(arg);
         }
 
-        s = subprocess(parse->value(), args, Subprocess::PATH("/dev/null"));
+        status = os::spawn(parse->value(), args);
       }
 
-      if (s.isError()) {
-        cerr << "Failed to create the pre-exec subprocess: "
-             << s.error() << endl;
-        return EXIT_FAILURE;
-      }
-
-      s->status().await();
-
-      Option<int> status = s->status().get();
-      if (status.isNone()) {
-        cerr << "Failed to reap the pre-exec subprocess "
-             << "'" << value << "'" << endl;
-        return EXIT_FAILURE;
-      } else if (status.get() != 0) {
-        cerr << "The pre-exec subprocess '" << value << "' "
-             << "failed" << endl;
+      if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0)) {
+        cerr << "Failed to execute pre-exec command '" << value << "'" << endl;
         return EXIT_FAILURE;
       }
     }

Reply via email to