From 6de21f11125941ea1b94fdeec754bacea3916fd5 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 22 Aug 2017 15:41:03 -0700 Subject: [PATCH] init: cleanup environment handling Init keep its own copy of the environment that it uses for execve when starting services. This is unnecessary however as libc already has functions that mutate the environment and the environment that init uses is clean for starting services. This change removes init's copy of the environment and uses the libc functions instead. This also makes small clean-up to the way the Service class stores service specific environment variables. Test: boot bullhead Change-Id: I7c98a0b7aac9fa8f195ae33bd6a7515bb56faf78 --- init/builtins.cpp | 4 ++-- init/descriptors.cpp | 3 +-- init/init.cpp | 38 ++------------------------------------ init/init.h | 4 ---- init/selinux.cpp | 4 +--- init/service.cpp | 16 ++++------------ init/service.h | 9 +-------- 7 files changed, 11 insertions(+), 67 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index f807343ab..593f718f1 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -159,8 +159,8 @@ static Result do_exec_start(const std::vector& args) { } static Result do_export(const std::vector& args) { - if (!add_environment(args[1].c_str(), args[2].c_str())) { - return Error(); + if (setenv(args[1].c_str(), args[2].c_str(), 1) == -1) { + return ErrnoError() << "setenv() failed"; } return Success(); } diff --git a/init/descriptors.cpp b/init/descriptors.cpp index cc5b948e9..62656872f 100644 --- a/init/descriptors.cpp +++ b/init/descriptors.cpp @@ -28,7 +28,6 @@ #include #include -#include "init.h" #include "util.h" namespace android { @@ -62,7 +61,7 @@ void DescriptorInfo::CreateAndPublish(const std::string& globalContext) const { [] (char& c) { c = isalnum(c) ? c : '_'; }); std::string val = std::to_string(fd); - add_environment(publishedName.c_str(), val.c_str()); + setenv(publishedName.c_str(), val.c_str(), 1); // make sure we don't close on exec fcntl(fd, F_SETFD, 0); diff --git a/init/init.cpp b/init/init.cpp index d0afac11d..e1bd3a2cb 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -71,8 +71,6 @@ static char qemu[32]; std::string default_console = "/dev/console"; -const char *ENV[32]; - static int epoll_fd = -1; static std::unique_ptr waiting_for_prop(nullptr); @@ -126,38 +124,6 @@ void register_epoll_handler(int fd, void (*fn)()) { } } -/* add_environment - add "key=value" to the current environment */ -int add_environment(const char *key, const char *val) -{ - size_t n; - size_t key_len = strlen(key); - - /* The last environment entry is reserved to terminate the list */ - for (n = 0; n < (arraysize(ENV) - 1); n++) { - - /* Delete any existing entry for this key */ - if (ENV[n] != NULL) { - size_t entry_key_len = strcspn(ENV[n], "="); - if ((entry_key_len == key_len) && (strncmp(ENV[n], key, entry_key_len) == 0)) { - free((char*)ENV[n]); - ENV[n] = NULL; - } - } - - /* Add entry if a free slot is available */ - if (ENV[n] == NULL) { - char* entry; - asprintf(&entry, "%s=%s", key, val); - ENV[n] = entry; - return 0; - } - } - - LOG(ERROR) << "No env. room to store: '" << key << "':'" << val << "'"; - - return -1; -} - bool start_waiting_for_property(const char *name, const char *value) { if (waiting_for_prop) { @@ -429,8 +395,6 @@ int main(int argc, char** argv) { install_reboot_signal_handlers(); } - add_environment("PATH", _PATH_DEFPATH); - bool is_first_stage = (getenv("INIT_SECOND_STAGE") == nullptr); if (is_first_stage) { @@ -439,6 +403,8 @@ int main(int argc, char** argv) { // Clear the umask. umask(0); + clearenv(); + setenv("PATH", _PATH_DEFPATH, 1); // Get the basic filesystem setup we need put together in the initramdisk // on / and then we'll let the rc file figure out the rest. mount("tmpfs", "/dev", "tmpfs", MS_NOSUID, "mode=0755"); diff --git a/init/init.h b/init/init.h index 50a7c8352..b757c1d65 100644 --- a/init/init.h +++ b/init/init.h @@ -30,9 +30,7 @@ namespace init { // Note: These globals are *only* valid in init, so they should not be used in ueventd, // watchdogd, or any files that may be included in those, such as devices.cpp and util.cpp. // TODO: Have an Init class and remove all globals. -extern const char *ENV[32]; extern std::string default_console; - extern std::vector late_import_paths; Parser CreateParser(ActionManager& action_manager, ServiceList& service_list); @@ -43,8 +41,6 @@ void property_changed(const std::string& name, const std::string& value); void register_epoll_handler(int fd, void (*fn)()); -int add_environment(const char* key, const char* val); - bool start_waiting_for_property(const char *name, const char *value); void DumpState(); diff --git a/init/selinux.cpp b/init/selinux.cpp index c8240286d..ef59164e3 100644 --- a/init/selinux.cpp +++ b/init/selinux.cpp @@ -48,7 +48,6 @@ #include "selinux.h" #include -#include #include #include #include @@ -126,8 +125,7 @@ bool ForkExecveAndWaitForCompletion(const char* filename, char* const argv[]) { } TEMP_FAILURE_RETRY(close(pipe_fds[1])); - const char* envp[] = {_PATH_DEFPATH, nullptr}; - if (execve(filename, argv, (char**)envp) == -1) { + if (execv(filename, argv) == -1) { PLOG(ERROR) << "Failed to execve " << filename; return false; } diff --git a/init/service.cpp b/init/service.cpp index 6ab60e3ca..de9538fbe 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -147,14 +147,6 @@ static void ExpandArgs(const std::vector& args, std::vector* strs->push_back(nullptr); } -ServiceEnvironmentInfo::ServiceEnvironmentInfo() { -} - -ServiceEnvironmentInfo::ServiceEnvironmentInfo(const std::string& name, - const std::string& value) - : name(name), value(value) { -} - unsigned long Service::next_start_order_ = 1; bool Service::is_exec_service_running_ = false; @@ -507,7 +499,7 @@ Result Service::ParseSeclabel(const std::vector& args) { } Result Service::ParseSetenv(const std::vector& args) { - envvars_.emplace_back(args[1], args[2]); + environment_vars_.emplace_back(args[1], args[2]); return Success(); } @@ -723,8 +715,8 @@ bool Service::Start() { SetUpPidNamespace(name_); } - for (const auto& ei : envvars_) { - add_environment(ei.name.c_str(), ei.value.c_str()); + for (const auto& [key, value] : environment_vars_) { + setenv(key.c_str(), value.c_str(), 1); } std::for_each(descriptors_.begin(), descriptors_.end(), @@ -779,7 +771,7 @@ bool Service::Start() { std::vector strs; ExpandArgs(args_, &strs); - if (execve(strs[0], (char**) &strs[0], (char**) ENV) < 0) { + if (execv(strs[0], (char**)&strs[0]) < 0) { PLOG(ERROR) << "cannot execve('" << strs[0] << "')"; } diff --git a/init/service.h b/init/service.h index fe851299e..be173cd64 100644 --- a/init/service.h +++ b/init/service.h @@ -57,13 +57,6 @@ namespace android { namespace init { -struct ServiceEnvironmentInfo { - ServiceEnvironmentInfo(); - ServiceEnvironmentInfo(const std::string& name, const std::string& value); - std::string name; - std::string value; -}; - class Service { public: Service(const std::string& name, const std::vector& args); @@ -178,7 +171,7 @@ class Service { std::string seclabel_; std::vector> descriptors_; - std::vector envvars_; + std::vector> environment_vars_; Action onrestart_; // Commands to execute on restart.