diff --git a/init/Android.bp b/init/Android.bp index 3bb08db6c..52628f372 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -28,6 +28,7 @@ init_common_sources = [ "rlimit_parser.cpp", "service.cpp", "service_list.cpp", + "service_lock.cpp", "service_parser.cpp", "service_utils.cpp", "subcontext.cpp", @@ -81,6 +82,7 @@ cc_defaults { "-Wextra", "-Wno-unused-parameter", "-Werror", + "-Wthread-safety", "-DALLOW_FIRST_STAGE_CONSOLE=0", "-DALLOW_LOCAL_PROP_OVERRIDE=0", "-DALLOW_PERMISSIVE_SELINUX=0", diff --git a/init/action_manager.cpp b/init/action_manager.cpp index ebca762ca..b45f5cd18 100644 --- a/init/action_manager.cpp +++ b/init/action_manager.cpp @@ -41,10 +41,12 @@ void ActionManager::AddAction(std::unique_ptr action) { } void ActionManager::QueueEventTrigger(const std::string& trigger) { + auto lock = std::lock_guard{event_queue_lock_}; event_queue_.emplace(trigger); } void ActionManager::QueuePropertyChange(const std::string& name, const std::string& value) { + auto lock = std::lock_guard{event_queue_lock_}; event_queue_.emplace(std::make_pair(name, value)); } @@ -53,6 +55,7 @@ void ActionManager::QueueAllPropertyActions() { } void ActionManager::QueueBuiltinAction(BuiltinFunction func, const std::string& name) { + auto lock = std::lock_guard{event_queue_lock_}; auto action = std::make_unique(true, nullptr, "", 0, name, std::map{}); action->AddCommand(std::move(func), {name}, 0); @@ -62,15 +65,18 @@ void ActionManager::QueueBuiltinAction(BuiltinFunction func, const std::string& } void ActionManager::ExecuteOneCommand() { - // Loop through the event queue until we have an action to execute - while (current_executing_actions_.empty() && !event_queue_.empty()) { - for (const auto& action : actions_) { - if (std::visit([&action](const auto& event) { return action->CheckEvent(event); }, - event_queue_.front())) { - current_executing_actions_.emplace(action.get()); + { + auto lock = std::lock_guard{event_queue_lock_}; + // Loop through the event queue until we have an action to execute + while (current_executing_actions_.empty() && !event_queue_.empty()) { + for (const auto& action : actions_) { + if (std::visit([&action](const auto& event) { return action->CheckEvent(event); }, + event_queue_.front())) { + current_executing_actions_.emplace(action.get()); + } } + event_queue_.pop(); } - event_queue_.pop(); } if (current_executing_actions_.empty()) { @@ -103,6 +109,7 @@ void ActionManager::ExecuteOneCommand() { } bool ActionManager::HasMoreCommands() const { + auto lock = std::lock_guard{event_queue_lock_}; return !current_executing_actions_.empty() || !event_queue_.empty(); } @@ -113,6 +120,7 @@ void ActionManager::DumpState() const { } void ActionManager::ClearQueue() { + auto lock = std::lock_guard{event_queue_lock_}; // We are shutting down so don't claim the oneshot builtin actions back current_executing_actions_ = {}; event_queue_ = {}; diff --git a/init/action_manager.h b/init/action_manager.h index a2b95acad..b6f93d9b5 100644 --- a/init/action_manager.h +++ b/init/action_manager.h @@ -16,9 +16,12 @@ #pragma once +#include #include #include +#include + #include "action.h" #include "builtins.h" @@ -48,7 +51,9 @@ class ActionManager { void operator=(ActionManager const&) = delete; std::vector> actions_; - std::queue> event_queue_; + std::queue> event_queue_ + GUARDED_BY(event_queue_lock_); + mutable std::mutex event_queue_lock_; std::queue current_executing_actions_; std::size_t current_command_; }; diff --git a/init/builtins.cpp b/init/builtins.cpp index 200bfff7d..dd5af72b1 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -151,6 +151,7 @@ static Result reboot_into_recovery(const std::vector& options template static void ForEachServiceInClass(const std::string& classname, F function) { + auto lock = std::lock_guard{service_lock}; for (const auto& service : ServiceList::GetInstance()) { if (service->classnames().count(classname)) std::invoke(function, service); } @@ -162,6 +163,7 @@ static Result do_class_start(const BuiltinArguments& args) { return {}; // Starting a class does not start services which are explicitly disabled. // They must be started individually. + auto lock = std::lock_guard{service_lock}; for (const auto& service : ServiceList::GetInstance()) { if (service->classnames().count(args[1])) { if (auto result = service->StartIfNotDisabled(); !result.ok()) { @@ -184,6 +186,7 @@ static Result do_class_start_post_data(const BuiltinArguments& args) { // stopped either. return {}; } + auto lock = std::lock_guard{service_lock}; for (const auto& service : ServiceList::GetInstance()) { if (service->classnames().count(args[1])) { if (auto result = service->StartIfPostData(); !result.ok()) { @@ -234,6 +237,7 @@ static Result do_domainname(const BuiltinArguments& args) { } static Result do_enable(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) return Error() << "Could not find service"; @@ -245,6 +249,7 @@ static Result do_enable(const BuiltinArguments& args) { } static Result do_exec(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; auto service = Service::MakeTemporaryOneshotService(args.args); if (!service.ok()) { return Error() << "Could not create exec service: " << service.error(); @@ -258,6 +263,7 @@ static Result do_exec(const BuiltinArguments& args) { } static Result do_exec_background(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; auto service = Service::MakeTemporaryOneshotService(args.args); if (!service.ok()) { return Error() << "Could not create exec background service: " << service.error(); @@ -271,6 +277,7 @@ static Result do_exec_background(const BuiltinArguments& args) { } static Result do_exec_start(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* service = ServiceList::GetInstance().FindService(args[1]); if (!service) { return Error() << "Service not found"; @@ -340,6 +347,7 @@ static Result do_insmod(const BuiltinArguments& args) { } static Result do_interface_restart(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindInterface(args[1]); if (!svc) return Error() << "interface " << args[1] << " not found"; svc->Restart(); @@ -347,6 +355,7 @@ static Result do_interface_restart(const BuiltinArguments& args) { } static Result do_interface_start(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindInterface(args[1]); if (!svc) return Error() << "interface " << args[1] << " not found"; if (auto result = svc->Start(); !result.ok()) { @@ -356,6 +365,7 @@ static Result do_interface_start(const BuiltinArguments& args) { } static Result do_interface_stop(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindInterface(args[1]); if (!svc) return Error() << "interface " << args[1] << " not found"; svc->Stop(); @@ -740,6 +750,7 @@ static Result do_setrlimit(const BuiltinArguments& args) { } static Result do_start(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) return Error() << "service " << args[1] << " not found"; if (auto result = svc->Start(); !result.ok()) { @@ -749,6 +760,7 @@ static Result do_start(const BuiltinArguments& args) { } static Result do_stop(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) return Error() << "service " << args[1] << " not found"; svc->Stop(); @@ -756,6 +768,7 @@ static Result do_stop(const BuiltinArguments& args) { } static Result do_restart(const BuiltinArguments& args) { + auto lock = std::lock_guard{service_lock}; Service* svc = ServiceList::GetInstance().FindService(args[1]); if (!svc) return Error() << "service " << args[1] << " not found"; svc->Restart(); @@ -1111,6 +1124,7 @@ static Result ExecWithFunctionOnFailure(const std::vector& ar function(StringPrintf("Exec service failed, status %d", siginfo.si_status)); } }); + auto lock = std::lock_guard{service_lock}; if (auto result = (*service)->ExecStart(); !result.ok()) { function("ExecStart failed: " + result.error().message()); } @@ -1250,6 +1264,7 @@ static Result parse_apex_configs() { } success &= parser.ParseConfigFile(c); } + auto lock = std::lock_guard{service_lock}; ServiceList::GetInstance().MarkServicesUpdate(); if (success) { return {}; diff --git a/init/init.cpp b/init/init.cpp index 5bf1b36c9..bfef9e5bc 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -33,7 +33,9 @@ #include #include #include +#include #include +#include #include #include @@ -95,15 +97,148 @@ static int property_triggers_enabled = 0; static int signal_fd = -1; static int property_fd = -1; -static std::unique_ptr waiting_for_prop(nullptr); -static std::string wait_prop_name; -static std::string wait_prop_value; -static std::string shutdown_command; -static bool do_shutdown = false; - static std::unique_ptr subcontext; +// Init epolls various FDs to wait for various inputs. It previously waited on property changes +// with a blocking socket that contained the information related to the change, however, it was easy +// to fill that socket and deadlock the system. Now we use locks to handle the property changes +// directly in the property thread, however we still must wake the epoll to inform init that there +// is a change to process, so we use this FD. It is non-blocking, since we do not care how many +// times WakeEpoll() is called, only that the epoll will wake. +static int wake_epoll_fd = -1; +static void InstallInitNotifier(Epoll* epoll) { + int sockets[2]; + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, sockets) != 0) { + PLOG(FATAL) << "Failed to socketpair() between property_service and init"; + } + int epoll_fd = sockets[0]; + wake_epoll_fd = sockets[1]; + + auto drain_socket = [epoll_fd] { + char buf[512]; + while (read(epoll_fd, buf, sizeof(buf)) > 0) { + } + }; + + if (auto result = epoll->RegisterHandler(epoll_fd, drain_socket); !result) { + LOG(FATAL) << result.error(); + } +} + +static void WakeEpoll() { + constexpr char value[] = "1"; + write(wake_epoll_fd, value, sizeof(value)); +} + +static class PropWaiterState { + public: + bool StartWaiting(const char* name, const char* value) { + auto lock = std::lock_guard{lock_}; + if (waiting_for_prop_) { + return false; + } + if (GetProperty(name, "") != value) { + // Current property value is not equal to expected value + wait_prop_name_ = name; + wait_prop_value_ = value; + waiting_for_prop_.reset(new Timer()); + } else { + LOG(INFO) << "start_waiting_for_property(\"" << name << "\", \"" << value + << "\"): already set"; + } + return true; + } + + void ResetWaitForProp() { + auto lock = std::lock_guard{lock_}; + ResetWaitForPropLocked(); + } + + void CheckAndResetWait(const std::string& name, const std::string& value) { + auto lock = std::lock_guard{lock_}; + // We always record how long init waited for ueventd to tell us cold boot finished. + // If we aren't waiting on this property, it means that ueventd finished before we even + // started to wait. + if (name == kColdBootDoneProp) { + auto time_waited = waiting_for_prop_ ? waiting_for_prop_->duration().count() : 0; + std::thread([time_waited] { + SetProperty("ro.boottime.init.cold_boot_wait", std::to_string(time_waited)); + }).detach(); + } + + if (waiting_for_prop_) { + if (wait_prop_name_ == name && wait_prop_value_ == value) { + LOG(INFO) << "Wait for property '" << wait_prop_name_ << "=" << wait_prop_value_ + << "' took " << *waiting_for_prop_; + ResetWaitForPropLocked(); + WakeEpoll(); + } + } + } + + // This is not thread safe because it releases the lock when it returns, so the waiting state + // may change. However, we only use this function to prevent running commands in the main + // thread loop when we are waiting, so we do not care about false positives; only false + // negatives. StartWaiting() and this function are always called from the same thread, so false + // negatives are not possible and therefore we're okay. + bool MightBeWaiting() { + auto lock = std::lock_guard{lock_}; + return static_cast(waiting_for_prop_); + } + + private: + void ResetWaitForPropLocked() { + wait_prop_name_.clear(); + wait_prop_value_.clear(); + waiting_for_prop_.reset(); + } + + std::mutex lock_; + std::unique_ptr waiting_for_prop_{nullptr}; + std::string wait_prop_name_; + std::string wait_prop_value_; + +} prop_waiter_state; + +bool start_waiting_for_property(const char* name, const char* value) { + return prop_waiter_state.StartWaiting(name, value); +} + +void ResetWaitForProp() { + prop_waiter_state.ResetWaitForProp(); +} + +static class ShutdownState { + public: + void TriggerShutdown(const std::string& command) { + // We can't call HandlePowerctlMessage() directly in this function, + // because it modifies the contents of the action queue, which can cause the action queue + // to get into a bad state if this function is called from a command being executed by the + // action queue. Instead we set this flag and ensure that shutdown happens before the next + // command is run in the main init loop. + auto lock = std::lock_guard{shutdown_command_lock_}; + shutdown_command_ = command; + do_shutdown_ = true; + WakeEpoll(); + } + + std::optional CheckShutdown() { + auto lock = std::lock_guard{shutdown_command_lock_}; + if (do_shutdown_ && !IsShuttingDown()) { + do_shutdown_ = false; + return shutdown_command_; + } + return {}; + } + + private: + std::mutex shutdown_command_lock_; + std::string shutdown_command_; + bool do_shutdown_ = false; +} shutdown_state; + void DumpState() { + auto lock = std::lock_guard{service_lock}; ServiceList::GetInstance().DumpState(); ActionManager::GetInstance().DumpState(); } @@ -156,40 +291,7 @@ static void LoadBootScripts(ActionManager& action_manager, ServiceList& service_ } } -bool start_waiting_for_property(const char *name, const char *value) -{ - if (waiting_for_prop) { - return false; - } - if (GetProperty(name, "") != value) { - // Current property value is not equal to expected value - wait_prop_name = name; - wait_prop_value = value; - waiting_for_prop.reset(new Timer()); - } else { - LOG(INFO) << "start_waiting_for_property(\"" - << name << "\", \"" << value << "\"): already set"; - } - return true; -} - -void ResetWaitForProp() { - wait_prop_name.clear(); - wait_prop_value.clear(); - waiting_for_prop.reset(); -} - -static void TriggerShutdown(const std::string& command) { - // We can't call HandlePowerctlMessage() directly in this function, - // because it modifies the contents of the action queue, which can cause the action queue - // to get into a bad state if this function is called from a command being executed by the - // action queue. Instead we set this flag and ensure that shutdown happens before the next - // command is run in the main init loop. - shutdown_command = command; - do_shutdown = true; -} - -void property_changed(const std::string& name, const std::string& value) { +void PropertyChanged(const std::string& name, const std::string& value) { // If the property is sys.powerctl, we bypass the event queue and immediately handle it. // This is to ensure that init will always and immediately shutdown/reboot, regardless of // if there are other pending events to process or if init is waiting on an exec service or @@ -197,30 +299,20 @@ void property_changed(const std::string& name, const std::string& value) { // In non-thermal-shutdown case, 'shutdown' trigger will be fired to let device specific // commands to be executed. if (name == "sys.powerctl") { - TriggerShutdown(value); + trigger_shutdown(value); } - if (property_triggers_enabled) ActionManager::GetInstance().QueuePropertyChange(name, value); - - // We always record how long init waited for ueventd to tell us cold boot finished. - // If we aren't waiting on this property, it means that ueventd finished before we even started - // to wait. - if (name == kColdBootDoneProp) { - auto time_waited = waiting_for_prop ? waiting_for_prop->duration().count() : 0; - SetProperty("ro.boottime.init.cold_boot_wait", std::to_string(time_waited)); + if (property_triggers_enabled) { + ActionManager::GetInstance().QueuePropertyChange(name, value); + WakeEpoll(); } - if (waiting_for_prop) { - if (wait_prop_name == name && wait_prop_value == value) { - LOG(INFO) << "Wait for property '" << wait_prop_name << "=" << wait_prop_value - << "' took " << *waiting_for_prop; - ResetWaitForProp(); - } - } + prop_waiter_state.CheckAndResetWait(name, value); } static std::optional HandleProcessActions() { std::optional next_process_action_time; + auto lock = std::lock_guard{service_lock}; for (const auto& s : ServiceList::GetInstance()) { if ((s->flags() & SVC_RUNNING) && s->timeout_period()) { auto timeout_time = s->time_started() + *s->timeout_period(); @@ -249,7 +341,7 @@ static std::optional HandleProcessActions() { return next_process_action_time; } -static Result DoControlStart(Service* service) { +static Result DoControlStart(Service* service) REQUIRES(service_lock) { return service->Start(); } @@ -258,7 +350,7 @@ static Result DoControlStop(Service* service) { return {}; } -static Result DoControlRestart(Service* service) { +static Result DoControlRestart(Service* service) REQUIRES(service_lock) { service->Restart(); return {}; } @@ -292,7 +384,7 @@ static const std::map& get_control_message_ return control_message_functions; } -bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t pid) { +bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t from_pid) { const auto& map = get_control_message_map(); const auto it = map.find(msg); @@ -301,7 +393,7 @@ bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t return false; } - std::string cmdline_path = StringPrintf("proc/%d/cmdline", pid); + std::string cmdline_path = StringPrintf("proc/%d/cmdline", from_pid); std::string process_cmdline; if (ReadFileToString(cmdline_path, &process_cmdline)) { std::replace(process_cmdline.begin(), process_cmdline.end(), '\0', ' '); @@ -312,6 +404,8 @@ bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t const ControlMessageFunction& function = it->second; + auto lock = std::lock_guard{service_lock}; + Service* svc = nullptr; switch (function.target) { @@ -329,23 +423,24 @@ bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t if (svc == nullptr) { LOG(ERROR) << "Control message: Could not find '" << name << "' for ctl." << msg - << " from pid: " << pid << " (" << process_cmdline << ")"; + << " from pid: " << from_pid << " (" << process_cmdline << ")"; return false; } if (auto result = function.action(svc); !result.ok()) { LOG(ERROR) << "Control message: Could not ctl." << msg << " for '" << name - << "' from pid: " << pid << " (" << process_cmdline << "): " << result.error(); + << "' from pid: " << from_pid << " (" << process_cmdline + << "): " << result.error(); return false; } LOG(INFO) << "Control message: Processed ctl." << msg << " for '" << name - << "' from pid: " << pid << " (" << process_cmdline << ")"; + << "' from pid: " << from_pid << " (" << process_cmdline << ")"; return true; } static Result wait_for_coldboot_done_action(const BuiltinArguments& args) { - if (!start_waiting_for_property(kColdBootDoneProp, "true")) { + if (!prop_waiter_state.StartWaiting(kColdBootDoneProp, "true")) { LOG(FATAL) << "Could not wait for '" << kColdBootDoneProp << "'"; } @@ -493,6 +588,7 @@ void HandleKeychord(const std::vector& keycodes) { } auto found = false; + auto lock = std::lock_guard{service_lock}; for (const auto& service : ServiceList::GetInstance()) { auto svc = service.get(); if (svc->keycodes() == keycodes) { @@ -579,44 +675,6 @@ void SendStartSendingMessagesMessage() { } } -static void HandlePropertyFd() { - auto message = ReadMessage(property_fd); - if (!message.ok()) { - LOG(ERROR) << "Could not read message from property service: " << message.error(); - return; - } - - auto property_message = PropertyMessage{}; - if (!property_message.ParseFromString(*message)) { - LOG(ERROR) << "Could not parse message from property service"; - return; - } - - switch (property_message.msg_case()) { - case PropertyMessage::kControlMessage: { - auto& control_message = property_message.control_message(); - bool success = HandleControlMessage(control_message.msg(), control_message.name(), - control_message.pid()); - - uint32_t response = success ? PROP_SUCCESS : PROP_ERROR_HANDLE_CONTROL_MESSAGE; - if (control_message.has_fd()) { - int fd = control_message.fd(); - TEMP_FAILURE_RETRY(send(fd, &response, sizeof(response), 0)); - close(fd); - } - break; - } - case PropertyMessage::kChangedMessage: { - auto& changed_message = property_message.changed_message(); - property_changed(changed_message.name(), changed_message.value()); - break; - } - default: - LOG(ERROR) << "Unknown message type from property service: " - << property_message.msg_case(); - } -} - int SecondStageMain(int argc, char** argv) { if (REBOOT_BOOTLOADER_ON_PANIC) { InstallRebootSignalHandlers(); @@ -624,7 +682,7 @@ int SecondStageMain(int argc, char** argv) { boot_clock::time_point start_time = boot_clock::now(); - trigger_shutdown = TriggerShutdown; + trigger_shutdown = [](const std::string& command) { shutdown_state.TriggerShutdown(command); }; SetStdioToDevNull(argv); InitKernelLogging(argv); @@ -684,11 +742,8 @@ int SecondStageMain(int argc, char** argv) { } InstallSignalFdHandler(&epoll); - + InstallInitNotifier(&epoll); StartPropertyService(&property_fd); - if (auto result = epoll.RegisterHandler(property_fd, HandlePropertyFd); !result.ok()) { - LOG(FATAL) << "Could not register epoll handler for property fd: " << result.error(); - } // Make the time that init stages started available for bootstat to log. RecordStageBoottimes(start_time); @@ -742,6 +797,7 @@ int SecondStageMain(int argc, char** argv) { Keychords keychords; am.QueueBuiltinAction( [&epoll, &keychords](const BuiltinArguments& args) -> Result { + auto lock = std::lock_guard{service_lock}; for (const auto& svc : ServiceList::GetInstance()) { keychords.Register(svc->keycodes()); } @@ -772,12 +828,12 @@ int SecondStageMain(int argc, char** argv) { // By default, sleep until something happens. auto epoll_timeout = std::optional{}; - if (do_shutdown && !IsShuttingDown()) { - do_shutdown = false; - HandlePowerctlMessage(shutdown_command); + auto shutdown_command = shutdown_state.CheckShutdown(); + if (shutdown_command) { + HandlePowerctlMessage(*shutdown_command); } - if (!(waiting_for_prop || Service::is_exec_service_running())) { + if (!(prop_waiter_state.MightBeWaiting() || Service::is_exec_service_running())) { am.ExecuteOneCommand(); } if (!IsShuttingDown()) { @@ -791,7 +847,7 @@ int SecondStageMain(int argc, char** argv) { } } - if (!(waiting_for_prop || Service::is_exec_service_running())) { + if (!(prop_waiter_state.MightBeWaiting() || Service::is_exec_service_running())) { // If there's more work to do, wake up again immediately. if (am.HasMoreCommands()) epoll_timeout = 0ms; } diff --git a/init/init.h b/init/init.h index 4bbca6f3c..bcf24e73f 100644 --- a/init/init.h +++ b/init/init.h @@ -41,6 +41,9 @@ void SendLoadPersistentPropertiesMessage(); void SendStopSendingMessagesMessage(); void SendStartSendingMessagesMessage(); +void PropertyChanged(const std::string& name, const std::string& value); +bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t from_pid); + int SecondStageMain(int argc, char** argv); } // namespace init diff --git a/init/init_test.cpp b/init/init_test.cpp index caf3e0343..3053bd848 100644 --- a/init/init_test.cpp +++ b/init/init_test.cpp @@ -167,6 +167,7 @@ service A something ServiceList service_list; TestInitText(init_script, BuiltinFunctionMap(), {}, &service_list); + auto lock = std::lock_guard{service_lock}; ASSERT_EQ(1, std::distance(service_list.begin(), service_list.end())); auto service = service_list.begin()->get(); diff --git a/init/lmkd_service.cpp b/init/lmkd_service.cpp index dd1ab4d61..a531d0aea 100644 --- a/init/lmkd_service.cpp +++ b/init/lmkd_service.cpp @@ -79,7 +79,8 @@ static bool UnregisterProcess(pid_t pid) { } static void RegisterServices(pid_t exclude_pid) { - for (const auto& service : ServiceList::GetInstance().services()) { + auto lock = std::lock_guard{service_lock}; + for (const auto& service : ServiceList::GetInstance()) { auto svc = service.get(); if (svc->oom_score_adjust() != DEFAULT_OOM_SCORE_ADJUST) { // skip if process is excluded or not yet forked (pid==0) diff --git a/init/mount_namespace.cpp b/init/mount_namespace.cpp index 0749fe3b8..aa368492d 100644 --- a/init/mount_namespace.cpp +++ b/init/mount_namespace.cpp @@ -29,6 +29,7 @@ #include #include +#include "property_service.h" #include "util.h" namespace android { @@ -290,6 +291,14 @@ bool SwitchToDefaultMountNamespace() { return true; } if (default_ns_id != GetMountNamespaceId()) { + // The property service thread and its descendent threads must be in the correct mount + // namespace to call Service::Start(), however setns() only operates on a single thread and + // fails when secondary threads attempt to join the same mount namespace. Therefore, we + // must join the property service thread and its descendents before the setns() call. Those + // threads are then started again after the setns() call, and they'll be in the proper + // namespace. + PausePropertyService(); + if (setns(default_ns_fd.get(), CLONE_NEWNS) == -1) { PLOG(ERROR) << "Failed to switch back to the default mount namespace."; return false; @@ -299,6 +308,8 @@ bool SwitchToDefaultMountNamespace() { LOG(ERROR) << result.error(); return false; } + + ResumePropertyService(); } LOG(INFO) << "Switched to default mount namespace"; diff --git a/init/property_service.cpp b/init/property_service.cpp index 84644e818..b140ee438 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -92,8 +92,10 @@ namespace init { static bool persistent_properties_loaded = false; static int property_set_fd = -1; +static int from_init_socket = -1; static int init_socket = -1; static bool accept_messages = false; +static std::thread property_service_thread; static PropertyInfoAreaFile property_info_area; @@ -147,17 +149,6 @@ static bool CheckMacPerms(const std::string& name, const char* target_context, return has_access; } -static void SendPropertyChanged(const std::string& name, const std::string& value) { - auto property_msg = PropertyMessage{}; - auto* changed_message = property_msg.mutable_changed_message(); - changed_message->set_name(name); - changed_message->set_value(value); - - if (auto result = SendMessage(init_socket, property_msg); !result.ok()) { - LOG(ERROR) << "Failed to send property changed message: " << result.error(); - } -} - static uint32_t PropertySet(const std::string& name, const std::string& value, std::string* error) { size_t valuelen = value.size(); @@ -196,47 +187,137 @@ static uint32_t PropertySet(const std::string& name, const std::string& value, s // If init hasn't started its main loop, then it won't be handling property changed messages // anyway, so there's no need to try to send them. if (accept_messages) { - SendPropertyChanged(name, value); + PropertyChanged(name, value); } return PROP_SUCCESS; } -class AsyncRestorecon { +template +class SingleThreadExecutor { public: - void TriggerRestorecon(const std::string& path) { - auto guard = std::lock_guard{mutex_}; - paths_.emplace(path); + virtual ~SingleThreadExecutor() {} - if (!thread_started_) { - thread_started_ = true; - std::thread{&AsyncRestorecon::ThreadFunction, this}.detach(); + template + void Run(F&& item) { + auto guard = std::lock_guard{mutex_}; + items_.emplace(std::forward(item)); + + if (thread_state_ == ThreadState::kRunning || thread_state_ == ThreadState::kStopped) { + return; + } + + if (thread_state_ == ThreadState::kPendingJoin) { + thread_.join(); + } + + StartThread(); + } + + void StopAndJoin() { + auto lock = std::unique_lock{mutex_}; + if (thread_state_ == ThreadState::kPendingJoin) { + thread_.join(); + } else if (thread_state_ == ThreadState::kRunning) { + thread_state_ = ThreadState::kStopped; + lock.unlock(); + thread_.join(); + lock.lock(); + } + + thread_state_ = ThreadState::kStopped; + } + + void Restart() { + auto guard = std::lock_guard{mutex_}; + if (items_.empty()) { + thread_state_ = ThreadState::kNotStarted; + } else { + StartThread(); + } + } + + void MaybeJoin() { + auto guard = std::lock_guard{mutex_}; + if (thread_state_ == ThreadState::kPendingJoin) { + thread_.join(); + thread_state_ = ThreadState::kNotStarted; } } private: + virtual void Execute(T&& item) = 0; + + void StartThread() { + thread_state_ = ThreadState::kRunning; + auto thread = std::thread{&SingleThreadExecutor::ThreadFunction, this}; + std::swap(thread_, thread); + } + void ThreadFunction() { auto lock = std::unique_lock{mutex_}; - while (!paths_.empty()) { - auto path = paths_.front(); - paths_.pop(); + while (!items_.empty()) { + auto item = items_.front(); + items_.pop(); lock.unlock(); - if (selinux_android_restorecon(path.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE) != 0) { - LOG(ERROR) << "Asynchronous restorecon of '" << path << "' failed'"; - } - android::base::SetProperty(kRestoreconProperty, path); + Execute(std::move(item)); lock.lock(); } - thread_started_ = false; + if (thread_state_ != ThreadState::kStopped) { + thread_state_ = ThreadState::kPendingJoin; + } } std::mutex mutex_; - std::queue paths_; - bool thread_started_ = false; + std::queue items_; + enum class ThreadState { + kNotStarted, // Initial state when starting the program or when restarting with no items to + // process. + kRunning, // The thread is running and is in a state that it will process new items if + // are run. + kPendingJoin, // The thread has run to completion and is pending join(). A new thread must + // be launched for new items to be processed. + kStopped, // This executor has stopped and will not process more items until Restart() is + // called. Currently pending items will be processed and the thread will be + // joined. + }; + ThreadState thread_state_ = ThreadState::kNotStarted; + std::thread thread_; }; +class RestoreconThread : public SingleThreadExecutor { + virtual void Execute(std::string&& path) override { + if (selinux_android_restorecon(path.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE) != 0) { + LOG(ERROR) << "Asynchronous restorecon of '" << path << "' failed'"; + } + android::base::SetProperty(kRestoreconProperty, path); + } +}; + +struct ControlMessageInfo { + std::string message; + std::string name; + pid_t pid; + int fd; +}; + +class ControlMessageThread : public SingleThreadExecutor { + virtual void Execute(ControlMessageInfo&& info) override { + bool success = HandleControlMessage(info.message, info.name, info.pid); + + uint32_t response = success ? PROP_SUCCESS : PROP_ERROR_HANDLE_CONTROL_MESSAGE; + if (info.fd != -1) { + TEMP_FAILURE_RETRY(send(info.fd, &response, sizeof(response), 0)); + close(info.fd); + } + } +}; + +static RestoreconThread restorecon_thread; +static ControlMessageThread control_message_thread; + class SocketConnection { public: SocketConnection(int socket, const ucred& cred) : socket_(socket), cred_(cred) {} @@ -378,29 +459,17 @@ static uint32_t SendControlMessage(const std::string& msg, const std::string& na return PROP_ERROR_HANDLE_CONTROL_MESSAGE; } - auto property_msg = PropertyMessage{}; - auto* control_message = property_msg.mutable_control_message(); - control_message->set_msg(msg); - control_message->set_name(name); - control_message->set_pid(pid); - - // We must release the fd before sending it to init, otherwise there will be a race with init. - // If init calls close() before Release(), then fdsan will see the wrong tag and abort(). + // We must release the fd before spawning the thread, otherwise there will be a race with the + // thread. If the thread calls close() before this function calls Release(), then fdsan will see + // the wrong tag and abort(). int fd = -1; if (socket != nullptr && SelinuxGetVendorAndroidVersion() > __ANDROID_API_Q__) { fd = socket->Release(); - control_message->set_fd(fd); } - if (auto result = SendMessage(init_socket, property_msg); !result.ok()) { - // We've already released the fd above, so if we fail to send the message to init, we need - // to manually free it here. - if (fd != -1) { - close(fd); - } - *error = "Failed to send control message: " + result.error().message(); - return PROP_ERROR_HANDLE_CONTROL_MESSAGE; - } + // Handling a control message likely calls SetProperty, which we must synchronously handle, + // therefore we must fork a thread to handle it. + control_message_thread.Run({msg, name, pid, fd}); return PROP_SUCCESS; } @@ -502,8 +571,7 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, // We use a thread to do this restorecon operation to prevent holding up init, as it may take // a long time to complete. if (name == kRestoreconProperty && cr.pid != 1 && !value.empty()) { - static AsyncRestorecon async_restorecon; - async_restorecon.TriggerRestorecon(value); + restorecon_thread.Run(value); return PROP_SUCCESS; } @@ -1082,6 +1150,8 @@ void PropertyInit() { PropertyLoadBootDefaults(); } +static bool pause_property_service = false; + static void HandleInitSocket() { auto message = ReadMessage(init_socket); if (!message.ok()) { @@ -1116,6 +1186,10 @@ static void HandleInitSocket() { accept_messages = true; break; } + case InitMessage::kPausePropertyService: { + pause_property_service = true; + break; + } default: LOG(ERROR) << "Unknown message type from init: " << init_message.msg_case(); } @@ -1136,7 +1210,7 @@ static void PropertyServiceThread() { LOG(FATAL) << result.error(); } - while (true) { + while (!pause_property_service) { auto pending_functions = epoll.Wait(std::nullopt); if (!pending_functions.ok()) { LOG(ERROR) << pending_functions.error(); @@ -1145,9 +1219,34 @@ static void PropertyServiceThread() { (*function)(); } } + control_message_thread.MaybeJoin(); + restorecon_thread.MaybeJoin(); } } +void SendStopPropertyServiceMessage() { + auto init_message = InitMessage{}; + init_message.set_pause_property_service(true); + if (auto result = SendMessage(from_init_socket, init_message); !result.ok()) { + LOG(ERROR) << "Failed to send stop property service message: " << result.error(); + } +} + +void PausePropertyService() { + control_message_thread.StopAndJoin(); + restorecon_thread.StopAndJoin(); + SendStopPropertyServiceMessage(); + property_service_thread.join(); +} + +void ResumePropertyService() { + pause_property_service = false; + auto new_thread = std::thread{PropertyServiceThread}; + property_service_thread.swap(new_thread); + restorecon_thread.Restart(); + control_message_thread.Restart(); +} + void StartPropertyService(int* epoll_socket) { InitPropertySet("ro.property_service.version", "2"); @@ -1155,7 +1254,7 @@ void StartPropertyService(int* epoll_socket) { if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sockets) != 0) { PLOG(FATAL) << "Failed to socketpair() between property_service and init"; } - *epoll_socket = sockets[0]; + *epoll_socket = from_init_socket = sockets[0]; init_socket = sockets[1]; accept_messages = true; @@ -1169,7 +1268,8 @@ void StartPropertyService(int* epoll_socket) { listen(property_set_fd, 8); - std::thread{PropertyServiceThread}.detach(); + auto new_thread = std::thread{PropertyServiceThread}; + property_service_thread.swap(new_thread); } } // namespace init diff --git a/init/property_service.h b/init/property_service.h index 506d116e1..e92132632 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -31,6 +31,8 @@ bool CanReadProperty(const std::string& source_context, const std::string& name) void PropertyInit(); void StartPropertyService(int* epoll_socket); +void ResumePropertyService(); +void PausePropertyService(); } // namespace init } // namespace android diff --git a/init/property_service.proto b/init/property_service.proto index 08268d9bb..36245b228 100644 --- a/init/property_service.proto +++ b/init/property_service.proto @@ -41,5 +41,6 @@ message InitMessage { bool load_persistent_properties = 1; bool stop_sending_messages = 2; bool start_sending_messages = 3; + bool pause_property_service = 4; }; } diff --git a/init/reboot.cpp b/init/reboot.cpp index 048c1e7a7..f7cc36e14 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -85,7 +85,7 @@ static bool shutting_down = false; static const std::set kDebuggingServices{"tombstoned", "logd", "adbd", "console"}; -static std::vector GetDebuggingServices(bool only_post_data) { +static std::vector GetDebuggingServices(bool only_post_data) REQUIRES(service_lock) { std::vector ret; ret.reserve(kDebuggingServices.size()); for (const auto& s : ServiceList::GetInstance()) { @@ -179,7 +179,7 @@ class MountEntry { }; // Turn off backlight while we are performing power down cleanup activities. -static void TurnOffBacklight() { +static void TurnOffBacklight() REQUIRES(service_lock) { Service* service = ServiceList::GetInstance().FindService("blank_screen"); if (service == nullptr) { LOG(WARNING) << "cannot find blank_screen in TurnOffBacklight"; @@ -587,6 +587,7 @@ static void DoReboot(unsigned int cmd, const std::string& reason, const std::str // Start reboot monitor thread sem_post(&reboot_semaphore); + auto lock = std::lock_guard{service_lock}; // watchdogd is a vendor specific component but should be alive to complete shutdown safely. const std::set to_starts{"watchdogd"}; std::vector stop_first; @@ -706,6 +707,7 @@ static void EnterShutdown() { // Skip wait for prop if it is in progress ResetWaitForProp(); // Clear EXEC flag if there is one pending + auto lock = std::lock_guard{service_lock}; for (const auto& s : ServiceList::GetInstance()) { s->UnSetExec(); } @@ -749,6 +751,7 @@ static Result DoUserspaceReboot() { return Error() << "Failed to set sys.init.userspace_reboot.in_progress property"; } EnterShutdown(); + auto lock = std::lock_guard{service_lock}; if (!SetProperty("sys.powerctl", "")) { return Error() << "Failed to reset sys.powerctl property"; } @@ -907,6 +910,7 @@ void HandlePowerctlMessage(const std::string& command) { run_fsck = true; } else if (cmd_params[1] == "thermal") { // Turn off sources of heat immediately. + auto lock = std::lock_guard{service_lock}; TurnOffBacklight(); // run_fsck is false to avoid delay cmd = ANDROID_RB_THERMOFF; diff --git a/init/service.h b/init/service.h index cf3f0c290..d2a446207 100644 --- a/init/service.h +++ b/init/service.h @@ -27,12 +27,14 @@ #include #include +#include #include #include "action.h" #include "capabilities.h" #include "keyword_map.h" #include "parser.h" +#include "service_lock.h" #include "service_utils.h" #include "subcontext.h" @@ -77,17 +79,17 @@ class Service { bool IsRunning() { return (flags_ & SVC_RUNNING) != 0; } bool IsEnabled() { return (flags_ & SVC_DISABLED) == 0; } - Result ExecStart(); - Result Start(); - Result StartIfNotDisabled(); - Result StartIfPostData(); - Result Enable(); + Result ExecStart() REQUIRES(service_lock); + Result Start() REQUIRES(service_lock); + Result StartIfNotDisabled() REQUIRES(service_lock); + Result StartIfPostData() REQUIRES(service_lock); + Result Enable() REQUIRES(service_lock); void Reset(); void ResetIfPostData(); void Stop(); void Terminate(); void Timeout(); - void Restart(); + void Restart() REQUIRES(service_lock); void Reap(const siginfo_t& siginfo); void DumpState() const; void SetShutdownCritical() { flags_ |= SVC_SHUTDOWN_CRITICAL; } diff --git a/init/service_list.h b/init/service_list.h index 183862491..8cbc87888 100644 --- a/init/service_list.h +++ b/init/service_list.h @@ -17,9 +17,13 @@ #pragma once #include +#include #include +#include + #include "service.h" +#include "service_lock.h" namespace android { namespace init { @@ -32,16 +36,16 @@ class ServiceList { ServiceList(); size_t CheckAllCommands(); - void AddService(std::unique_ptr service); - void RemoveService(const Service& svc); + void AddService(std::unique_ptr service) REQUIRES(service_lock); + void RemoveService(const Service& svc) REQUIRES(service_lock); template - void RemoveServiceIf(UnaryPredicate predicate) { + void RemoveServiceIf(UnaryPredicate predicate) REQUIRES(service_lock) { services_.erase(std::remove_if(services_.begin(), services_.end(), predicate), services_.end()); } template - Service* FindService(T value, F function = &Service::name) const { + Service* FindService(T value, F function = &Service::name) const REQUIRES(service_lock) { auto svc = std::find_if(services_.begin(), services_.end(), [&function, &value](const std::unique_ptr& s) { return std::invoke(function, s) == value; @@ -52,7 +56,7 @@ class ServiceList { return nullptr; } - Service* FindInterface(const std::string& interface_name) { + Service* FindInterface(const std::string& interface_name) REQUIRES(service_lock) { for (const auto& svc : services_) { if (svc->interfaces().count(interface_name) > 0) { return svc.get(); @@ -62,18 +66,20 @@ class ServiceList { return nullptr; } - void DumpState() const; + void DumpState() const REQUIRES(service_lock); - auto begin() const { return services_.begin(); } - auto end() const { return services_.end(); } - const std::vector>& services() const { return services_; } - const std::vector services_in_shutdown_order() const; + auto begin() const REQUIRES(service_lock) { return services_.begin(); } + auto end() const REQUIRES(service_lock) { return services_.end(); } + const std::vector>& services() const REQUIRES(service_lock) { + return services_; + } + const std::vector services_in_shutdown_order() const REQUIRES(service_lock); void MarkPostData(); bool IsPostData(); - void MarkServicesUpdate(); + void MarkServicesUpdate() REQUIRES(service_lock); bool IsServicesUpdated() const { return services_update_finished_; } - void DelayService(const Service& service); + void DelayService(const Service& service) REQUIRES(service_lock); private: std::vector> services_; diff --git a/init/service_lock.cpp b/init/service_lock.cpp new file mode 100644 index 000000000..404d4396e --- /dev/null +++ b/init/service_lock.cpp @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "service_lock.h" + +namespace android { +namespace init { + +RecursiveMutex service_lock; + +} // namespace init +} // namespace android diff --git a/init/service_lock.h b/init/service_lock.h new file mode 100644 index 000000000..6b94271be --- /dev/null +++ b/init/service_lock.h @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include + +namespace android { +namespace init { + +// This class exists to add thread annotations, since they're absent from std::recursive_mutex. + +class CAPABILITY("mutex") RecursiveMutex { + public: + void lock() ACQUIRE() { mutex_.lock(); } + void unlock() RELEASE() { mutex_.unlock(); } + + private: + std::recursive_mutex mutex_; +}; + +extern RecursiveMutex service_lock; + +} // namespace init +} // namespace android diff --git a/init/service_parser.cpp b/init/service_parser.cpp index 560f693f9..51f4c9786 100644 --- a/init/service_parser.cpp +++ b/init/service_parser.cpp @@ -168,6 +168,7 @@ Result ServiceParser::ParseInterface(std::vector&& args) { const std::string fullname = interface_name + "/" + instance_name; + auto lock = std::lock_guard{service_lock}; for (const auto& svc : *service_list_) { if (svc->interfaces().count(fullname) > 0) { return Error() << "Interface '" << fullname << "' redefined in " << service_->name() @@ -598,6 +599,7 @@ Result ServiceParser::EndSection() { } } + auto lock = std::lock_guard{service_lock}; Service* old_service = service_list_->FindService(service_->name()); if (old_service) { if (!service_->is_override()) { diff --git a/init/sigchld_handler.cpp b/init/sigchld_handler.cpp index 9b2c7d939..064d64d54 100644 --- a/init/sigchld_handler.cpp +++ b/init/sigchld_handler.cpp @@ -64,6 +64,8 @@ static pid_t ReapOneProcess() { std::string wait_string; Service* service = nullptr; + auto lock = std::lock_guard{service_lock}; + if (SubcontextChildReap(pid)) { name = "Subcontext"; } else {