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/

*** Modified for 1.0.x ***


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

Branch: refs/heads/1.0.x
Commit: 08d871ddeeb1ed96fcb1544ebe2e8ddceb60164b
Parents: d73c57e
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 21:23:17 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/08d871dd/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp 
b/src/slave/containerizer/mesos/launch.cpp
index 2db8db5..e1853cb 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -22,8 +22,6 @@
 
 #include <iostream>
 
-#include <process/subprocess.hpp>
-
 #include <stout/foreach.hpp>
 #include <stout/os.hpp>
 #include <stout/protobuf.hpp>
@@ -44,8 +42,6 @@ using std::endl;
 using std::string;
 using std::vector;
 
-using process::Subprocess;
-
 namespace mesos {
 namespace internal {
 namespace slave {
@@ -219,37 +215,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