diff --git a/init/Android.bp b/init/Android.bp index 3bb08db6c..72a7bfed4 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -81,6 +81,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/init.cpp b/init/init.cpp index a7518fc2f..b29dfa3f6 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -33,7 +33,9 @@ #include #include #include +#include #include +#include #include #include @@ -95,14 +97,155 @@ 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; +struct PendingControlMessage { + std::string message; + std::string name; + pid_t pid; + int fd; +}; +static std::mutex pending_control_messages_lock; +static std::queue pending_control_messages; + +// 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.ok()) { + 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() { ServiceList::GetInstance().DumpState(); ActionManager::GetInstance().DumpState(); @@ -156,39 +299,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 @@ -196,26 +307,15 @@ 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() { @@ -343,8 +443,46 @@ bool HandleControlMessage(const std::string& msg, const std::string& name, pid_t return true; } +bool QueueControlMessage(const std::string& message, const std::string& name, pid_t pid, int fd) { + auto lock = std::lock_guard{pending_control_messages_lock}; + if (pending_control_messages.size() > 100) { + LOG(ERROR) << "Too many pending control messages, dropped '" << message << "' for '" << name + << "' from pid: " << pid; + return false; + } + pending_control_messages.push({message, name, pid, fd}); + WakeEpoll(); + return true; +} + +static void HandleControlMessages() { + auto lock = std::unique_lock{pending_control_messages_lock}; + // Init historically would only execute handle one property message, including control messages + // in each iteration of its main loop. We retain this behavior here to prevent starvation of + // other actions in the main loop. + if (!pending_control_messages.empty()) { + auto control_message = pending_control_messages.front(); + pending_control_messages.pop(); + lock.unlock(); + + bool success = HandleControlMessage(control_message.message, control_message.name, + control_message.pid); + + uint32_t response = success ? PROP_SUCCESS : PROP_ERROR_HANDLE_CONTROL_MESSAGE; + if (control_message.fd != -1) { + TEMP_FAILURE_RETRY(send(control_message.fd, &response, sizeof(response), 0)); + close(control_message.fd); + } + lock.lock(); + } + // If we still have items to process, make sure we wake back up to do so. + if (!pending_control_messages.empty()) { + WakeEpoll(); + } +} + 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 << "'"; } @@ -562,60 +700,6 @@ void SendLoadPersistentPropertiesMessage() { } } -void SendStopSendingMessagesMessage() { - auto init_message = InitMessage{}; - init_message.set_stop_sending_messages(true); - if (auto result = SendMessage(property_fd, init_message); !result.ok()) { - LOG(ERROR) << "Failed to send 'stop sending messages' message: " << result.error(); - } -} - -void SendStartSendingMessagesMessage() { - auto init_message = InitMessage{}; - init_message.set_start_sending_messages(true); - if (auto result = SendMessage(property_fd, init_message); !result.ok()) { - LOG(ERROR) << "Failed to send 'start sending messages' message: " << result.error(); - } -} - -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(); @@ -623,7 +707,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); @@ -683,11 +767,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); @@ -770,12 +851,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()) { @@ -789,7 +870,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; } @@ -806,6 +887,7 @@ int SecondStageMain(int argc, char** argv) { (*function)(); } } + HandleControlMessages(); } return 0; diff --git a/init/init.h b/init/init.h index 4bbca6f3c..27f64e297 100644 --- a/init/init.h +++ b/init/init.h @@ -38,8 +38,9 @@ void DumpState(); void ResetWaitForProp(); void SendLoadPersistentPropertiesMessage(); -void SendStopSendingMessagesMessage(); -void SendStartSendingMessagesMessage(); + +void PropertyChanged(const std::string& name, const std::string& value); +bool QueueControlMessage(const std::string& message, const std::string& name, pid_t pid, int fd); int SecondStageMain(int argc, char** argv); diff --git a/init/property_service.cpp b/init/property_service.cpp index 730bf6de4..820652249 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -92,8 +92,11 @@ 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::mutex accept_messages_lock; +static std::thread property_service_thread; static PropertyInfoAreaFile property_info_area; @@ -115,6 +118,16 @@ static int PropertyAuditCallback(void* data, security_class_t /*cls*/, char* buf return 0; } +void StartSendingMessages() { + auto lock = std::lock_guard{accept_messages_lock}; + accept_messages = true; +} + +void StopSendingMessages() { + auto lock = std::lock_guard{accept_messages_lock}; + accept_messages = true; +} + bool CanReadProperty(const std::string& source_context, const std::string& name) { const char* target_context = nullptr; property_info_area->GetPropertyInfo(name.c_str(), &target_context, nullptr); @@ -147,17 +160,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(); @@ -195,8 +197,9 @@ 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. + auto lock = std::lock_guard{accept_messages_lock}; if (accept_messages) { - SendPropertyChanged(name, value); + PropertyChanged(name, value); } return PROP_SUCCESS; } @@ -373,33 +376,24 @@ class SocketConnection { static uint32_t SendControlMessage(const std::string& msg, const std::string& name, pid_t pid, SocketConnection* socket, std::string* error) { + auto lock = std::lock_guard{accept_messages_lock}; if (!accept_messages) { *error = "Received control message after shutdown, ignoring"; 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(). 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; + bool queue_success = QueueControlMessage(msg, name, pid, fd); + if (!queue_success && fd != -1) { + uint32_t response = PROP_ERROR_HANDLE_CONTROL_MESSAGE; + TEMP_FAILURE_RETRY(send(fd, &response, sizeof(response), 0)); + close(fd); } return PROP_SUCCESS; @@ -1110,14 +1104,6 @@ static void HandleInitSocket() { persistent_properties_loaded = true; break; } - case InitMessage::kStopSendingMessages: { - accept_messages = false; - break; - } - case InitMessage::kStartSendingMessages: { - accept_messages = true; - break; - } default: LOG(ERROR) << "Unknown message type from init: " << init_message.msg_case(); } @@ -1157,9 +1143,9 @@ 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; + StartSendingMessages(); if (auto result = CreateSocket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, false, 0666, 0, 0, {}); @@ -1171,7 +1157,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..2d49a36fa 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -32,5 +32,8 @@ bool CanReadProperty(const std::string& source_context, const std::string& name) void PropertyInit(); void StartPropertyService(int* epoll_socket); +void StartSendingMessages(); +void StopSendingMessages(); + } // namespace init } // namespace android diff --git a/init/reboot.cpp b/init/reboot.cpp index 8d8bd8ee6..f006df3a3 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -59,6 +59,7 @@ #include "builtin_arguments.h" #include "init.h" #include "mount_namespace.h" +#include "property_service.h" #include "reboot_utils.h" #include "service.h" #include "service_list.h" @@ -711,17 +712,12 @@ static void EnterShutdown() { for (const auto& s : ServiceList::GetInstance()) { s->UnSetExec(); } - // We no longer process messages about properties changing coming from property service, so we - // need to tell property service to stop sending us these messages, otherwise it'll fill the - // buffers and block indefinitely, causing future property sets, including those that init makes - // during shutdown in Service::NotifyStateChange() to also block indefinitely. - SendStopSendingMessagesMessage(); } static void LeaveShutdown() { LOG(INFO) << "Leaving shutdown mode"; shutting_down = false; - SendStartSendingMessagesMessage(); + StartSendingMessages(); } static Result UnmountAllApexes() { @@ -981,6 +977,10 @@ void HandlePowerctlMessage(const std::string& command) { return; } + // We do not want to process any messages (queue'ing triggers, shutdown messages, control + // messages, etc) from properties during reboot. + StopSendingMessages(); + if (userspace_reboot) { HandleUserspaceReboot(); return;