diff options
| -rw-r--r-- | runtime/runtime.cc | 2 | ||||
| -rw-r--r-- | runtime/runtime.h | 14 | ||||
| -rw-r--r-- | runtime/utils.cc | 41 | ||||
| -rw-r--r-- | runtime/utils.h | 5 | ||||
| -rw-r--r-- | runtime/utils_test.cc | 51 |
5 files changed, 111 insertions, 2 deletions
diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 66c8f87fd1..21e664fc97 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -464,6 +464,8 @@ bool Runtime::Create(RuntimeArgumentMap&& runtime_options) { return false; } instance_ = new Runtime; + // Protect subprocesses from modifications to LD_LIBRARY_PATH, etc. + instance_->env_snapshot_.reset(art::TakeEnvSnapshot()); if (!instance_->Init(std::move(runtime_options))) { // TODO: Currently deleting the instance will abort the runtime on destruction. Now This will // leak memory, instead. Fix the destructor. b/19100793. diff --git a/runtime/runtime.h b/runtime/runtime.h index 1394462fd1..d0e8b4122c 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -96,6 +96,11 @@ class Transaction; typedef std::vector<std::pair<std::string, const void*>> RuntimeOptions; +struct EnvSnapshot { + public: + std::vector<std::unique_ptr<std::string> > name_value_pairs_; +}; + // Not all combinations of flags are valid. You may not visit all roots as well as the new roots // (no logical reason to do this). You also may not start logging new roots and stop logging new // roots (also no logical reason to do this). @@ -648,6 +653,12 @@ class Runtime { return zygote_no_threads_; } + // Returns a saved copy of the environment (getenv/setenv values). + // Used by Fork to protect against overwriting LD_LIBRARY_PATH, etc. + const EnvSnapshot* GetEnvSnapshot() const { + return env_snapshot_.get(); + } + private: static void InitPlatformSignalHandlers(); @@ -872,6 +883,9 @@ class Runtime { // Whether zygote code is in a section that should not start threads. bool zygote_no_threads_; + // Saved environment. + std::unique_ptr<const EnvSnapshot> env_snapshot_; + DISALLOW_COPY_AND_ASSIGN(Runtime); }; std::ostream& operator<<(std::ostream& os, const Runtime::CalleeSaveType& rhs); diff --git a/runtime/utils.cc b/runtime/utils.cc index 6a50b8eee2..fb6c1b3d3c 100644 --- a/runtime/utils.cc +++ b/runtime/utils.cc @@ -56,6 +56,22 @@ namespace art { +namespace { +#ifdef __APPLE__ +inline char** GetEnviron() { + // When Google Test is built as a framework on MacOS X, the environ variable + // is unavailable. Apple's documentation (man environ) recommends using + // _NSGetEnviron() instead. + return *_NSGetEnviron(); +} +#else +// Some POSIX platforms expect you to declare environ. extern "C" makes +// it reside in the global namespace. +extern "C" char** environ; +inline char** GetEnviron() { return environ; } +#endif +} // namespace + #if defined(__linux__) static constexpr bool kUseAddr2line = !kIsTargetBuild; #endif @@ -1393,6 +1409,15 @@ std::string GetSystemImageFilename(const char* location, const InstructionSet is return filename; } +const EnvSnapshot* TakeEnvSnapshot() { + EnvSnapshot* snapshot = new EnvSnapshot(); + char** env = GetEnviron(); + for (size_t i = 0; env[i] != nullptr; ++i) { + snapshot->name_value_pairs_.emplace_back(new std::string(env[i])); + } + return snapshot; +} + int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_msg) { const std::string command_line(Join(arg_vector, ' ')); CHECK_GE(arg_vector.size(), 1U) << command_line; @@ -1416,8 +1441,20 @@ int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_m // change process groups, so we don't get reaped by ProcessManager setpgid(0, 0); - execv(program, &args[0]); - PLOG(ERROR) << "Failed to execv(" << command_line << ")"; + // The child inherits the environment unless the caller overrides it. + if (Runtime::Current() == nullptr || Runtime::Current()->GetEnvSnapshot() == nullptr) { + execv(program, &args[0]); + } else { + const EnvSnapshot* saved_snapshot = Runtime::Current()->GetEnvSnapshot(); + // Allocation between fork and exec is not well-behaved. Use a variable-length array instead. + char* envp[saved_snapshot->name_value_pairs_.size() + 1]; + for (size_t i = 0; i < saved_snapshot->name_value_pairs_.size(); ++i) { + envp[i] = const_cast<char*>(saved_snapshot->name_value_pairs_[i]->c_str()); + } + envp[saved_snapshot->name_value_pairs_.size()] = nullptr; + execve(program, &args[0], envp); + } + PLOG(ERROR) << "Failed to execve(" << command_line << ")"; // _exit to avoid atexit handlers in child. _exit(1); } else { diff --git a/runtime/utils.h b/runtime/utils.h index c1e88a4feb..61db76a194 100644 --- a/runtime/utils.h +++ b/runtime/utils.h @@ -42,6 +42,7 @@ namespace art { class ArtField; class ArtMethod; class DexFile; +struct EnvSnapshot; namespace mirror { class Class; @@ -290,6 +291,10 @@ std::string GetDalvikCacheFilenameOrDie(const char* file_location, // Returns the system location for an image std::string GetSystemImageFilename(const char* location, InstructionSet isa); +// Capture an environment snapshot that will be used by Exec. +// Any subsequent setenvs will be ignored in child processes. +const EnvSnapshot* TakeEnvSnapshot(); + // Wrapper on fork/execv to run a command in a subprocess. bool Exec(std::vector<std::string>& arg_vector, std::string* error_msg); int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_msg); diff --git a/runtime/utils_test.cc b/runtime/utils_test.cc index f00edffab8..64f295e337 100644 --- a/runtime/utils_test.cc +++ b/runtime/utils_test.cc @@ -16,6 +16,8 @@ #include "utils.h" +#include <stdlib.h> + #include "class_linker-inl.h" #include "common_runtime_test.h" #include "mirror/array.h" @@ -379,6 +381,55 @@ TEST_F(UtilsTest, ExecError) { } } +TEST_F(UtilsTest, EnvSnapshotAdditionsAreNotVisible) { + static constexpr const char* kModifiedVariable = "EXEC_SHOULD_NOT_EXPORT_THIS"; + static constexpr int kOverwrite = 1; + // Set an variable in the current environment. + EXPECT_EQ(setenv(kModifiedVariable, "NEVER", kOverwrite), 0); + // Test that it is not exported. + std::vector<std::string> command; + if (kIsTargetBuild) { + std::string android_root(GetAndroidRoot()); + command.push_back(android_root + "/bin/printenv"); + } else { + command.push_back("/usr/bin/printenv"); + } + command.push_back(kModifiedVariable); + std::string error_msg; + if (!(RUNNING_ON_MEMORY_TOOL && kMemoryToolDetectsLeaks)) { + // Running on valgrind fails due to some memory that leaks in thread alternate signal stacks. + EXPECT_FALSE(Exec(command, &error_msg)); + EXPECT_NE(0U, error_msg.size()) << error_msg; + } +} + +TEST_F(UtilsTest, EnvSnapshotDeletionsAreNotVisible) { + static constexpr const char* kDeletedVariable = "PATH"; + static constexpr int kOverwrite = 1; + // Save the variable's value. + const char* save_value = getenv(kDeletedVariable); + EXPECT_NE(save_value, nullptr); + // Delete the variable. + EXPECT_EQ(unsetenv(kDeletedVariable), 0); + // Test that it is not exported. + std::vector<std::string> command; + if (kIsTargetBuild) { + std::string android_root(GetAndroidRoot()); + command.push_back(android_root + "/bin/printenv"); + } else { + command.push_back("/usr/bin/printenv"); + } + command.push_back(kDeletedVariable); + std::string error_msg; + if (!(RUNNING_ON_MEMORY_TOOL && kMemoryToolDetectsLeaks)) { + // Running on valgrind fails due to some memory that leaks in thread alternate signal stacks. + EXPECT_TRUE(Exec(command, &error_msg)); + EXPECT_EQ(0U, error_msg.size()) << error_msg; + } + // Restore the variable's value. + EXPECT_EQ(setenv(kDeletedVariable, save_value, kOverwrite), 0); +} + TEST_F(UtilsTest, IsValidDescriptor) { std::vector<uint8_t> descriptor( { 'L', 'a', '/', 'b', '$', 0xed, 0xa0, 0x80, 0xed, 0xb0, 0x80, ';', 0x00 }); |