From 3da2ba6d4aa16e5d087f98c1e8aa3e2e0820deef Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 28 Aug 2019 17:47:49 +0000 Subject: [PATCH] Revert "Reland: "init: run property service in a thread"" This reverts commit 8efca4bbb378ff5bd3af06d8511ea75a7ed49f99. Reason for revert: Still broken Change-Id: I3b37b1b00ff4b19f2eec2d8bd72042463d47cee3 --- init/Android.bp | 1 - init/builtins.cpp | 10 --- init/init.cpp | 51 +----------- init/init.h | 4 + init/property_service.cpp | 151 ++++++++++++------------------------ init/property_service.h | 7 +- init/property_service.proto | 37 --------- init/proto_utils.h | 62 --------------- init/subcontext.cpp | 64 +++++++++++++-- init/subcontext.proto | 6 ++ 10 files changed, 123 insertions(+), 270 deletions(-) delete mode 100644 init/property_service.proto delete mode 100644 init/proto_utils.h diff --git a/init/Android.bp b/init/Android.bp index 7dfb82894..285a6a44a 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -128,7 +128,6 @@ cc_library_static { "persistent_properties.cpp", "persistent_properties.proto", "property_service.cpp", - "property_service.proto", "property_type.cpp", "reboot.cpp", "reboot_utils.cpp", diff --git a/init/builtins.cpp b/init/builtins.cpp index 3573e34cf..e17e8993c 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -80,7 +80,6 @@ using namespace std::literals::string_literals; using android::base::Basename; -using android::base::StartsWith; using android::base::unique_fd; using android::fs_mgr::Fstab; using android::fs_mgr::ReadFstabFromFile; @@ -688,15 +687,6 @@ static Result do_swapon_all(const BuiltinArguments& args) { } static Result do_setprop(const BuiltinArguments& args) { - if (StartsWith(args[1], "ctl.")) { - return Error() << "InitPropertySet: Do not set ctl. properties from init; call the Service " - "functions directly"; - } - if (args[1] == kRestoreconProperty) { - return Error() << "InitPropertySet: Do not set '" << kRestoreconProperty - << "' from init; use the restorecon builtin directly"; - } - property_set(args[1], args[2]); return {}; } diff --git a/init/init.cpp b/init/init.cpp index e5d103640..18fb0c3ee 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -28,9 +28,6 @@ #include #include -#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_ -#include - #include #include #include @@ -64,7 +61,6 @@ #include "mount_handler.h" #include "mount_namespace.h" #include "property_service.h" -#include "proto_utils.h" #include "reboot.h" #include "reboot_utils.h" #include "security.h" @@ -73,7 +69,6 @@ #include "service.h" #include "service_parser.h" #include "sigchld_handler.h" -#include "system/core/init/property_service.pb.h" #include "util.h" using namespace std::chrono_literals; @@ -95,7 +90,6 @@ static int property_triggers_enabled = 0; static char qemu[32]; static int signal_fd = -1; -static int property_fd = -1; static std::unique_ptr waiting_for_prop(nullptr); static std::string wait_prop_name; @@ -619,44 +613,6 @@ static void RecordStageBoottimes(const boot_clock::time_point& second_stage_star selinux_start_time_ns)); } -static void HandlePropertyFd() { - auto message = ReadMessage(property_fd); - if (!message) { - 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(); @@ -728,12 +684,7 @@ int SecondStageMain(int argc, char** argv) { UmountDebugRamdisk(); fs_mgr_vendor_overlay_mount_all(); export_oem_lock_status(); - - StartPropertyService(&property_fd); - if (auto result = epoll.RegisterHandler(property_fd, HandlePropertyFd); !result) { - LOG(FATAL) << "Could not register epoll handler for property fd: " << result.error(); - } - + StartPropertyService(&epoll); MountHandler mount_handler(&epoll); set_usb_controller(); diff --git a/init/init.h b/init/init.h index 624a3d40d..cfc28f1be 100644 --- a/init/init.h +++ b/init/init.h @@ -31,6 +31,10 @@ namespace init { Parser CreateParser(ActionManager& action_manager, ServiceList& service_list); Parser CreateServiceOnlyParser(ServiceList& service_list); +bool HandleControlMessage(const std::string& msg, const std::string& arg, pid_t pid); + +void property_changed(const std::string& name, const std::string& value); + bool start_waiting_for_property(const char *name, const char *value); void DumpState(); diff --git a/init/property_service.cpp b/init/property_service.cpp index c78b81b2e..3408ff3be 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include @@ -64,10 +63,8 @@ #include "init.h" #include "persistent_properties.h" #include "property_type.h" -#include "proto_utils.h" #include "selinux.h" #include "subcontext.h" -#include "system/core/init/property_service.pb.h" #include "util.h" using namespace std::literals; @@ -79,7 +76,6 @@ using android::base::StartsWith; using android::base::StringPrintf; using android::base::Timer; using android::base::Trim; -using android::base::unique_fd; using android::base::WriteStringToFile; using android::properties::BuildTrie; using android::properties::ParsePropertyInfoFile; @@ -89,13 +85,18 @@ using android::properties::PropertyInfoEntry; namespace android { namespace init { +static constexpr const char kRestoreconProperty[] = "selinux.restorecon_recursive"; + static bool persistent_properties_loaded = false; static int property_set_fd = -1; -static int init_socket = -1; static PropertyInfoAreaFile property_info_area; +uint32_t InitPropertySet(const std::string& name, const std::string& value); + +uint32_t (*property_set)(const std::string& name, const std::string& value) = InitPropertySet; + void CreateSerializedPropertyInfo(); struct PropertyAuditData { @@ -163,17 +164,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) { - 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(); @@ -209,16 +199,7 @@ static uint32_t PropertySet(const std::string& name, const std::string& value, s if (persistent_properties_loaded && StartsWith(name, "persist.")) { WritePersistentProperty(name, value); } - // If init sets ro.persistent_properties.ready to true, then it has finished writing persistent - // properties, and we should write future persistent properties to disk. - if (name == "ro.persistent_properties.ready" && value == "true") { - persistent_properties_loaded = true; - } - // 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 (init_socket != -1) { - SendPropertyChanged(name, value); - } + property_changed(name, value); return PROP_SUCCESS; } @@ -258,10 +239,35 @@ class AsyncRestorecon { bool thread_started_ = false; }; +uint32_t InitPropertySet(const std::string& name, const std::string& value) { + if (StartsWith(name, "ctl.")) { + LOG(ERROR) << "InitPropertySet: Do not set ctl. properties from init; call the Service " + "functions directly"; + return PROP_ERROR_INVALID_NAME; + } + if (name == kRestoreconProperty) { + LOG(ERROR) << "InitPropertySet: Do not set '" << kRestoreconProperty + << "' from init; use the restorecon builtin directly"; + return PROP_ERROR_INVALID_NAME; + } + + uint32_t result = 0; + ucred cr = {.pid = 1, .uid = 0, .gid = 0}; + std::string error; + result = HandlePropertySet(name, value, kInitContext.c_str(), cr, &error); + if (result != PROP_SUCCESS) { + LOG(ERROR) << "Init cannot set '" << name << "' to '" << value << "': " << error; + } + + return result; +} + class SocketConnection { public: SocketConnection(int socket, const ucred& cred) : socket_(socket), cred_(cred) {} + ~SocketConnection() { close(socket_); } + bool RecvUint32(uint32_t* value, uint32_t* timeout_ms) { return RecvFully(value, sizeof(*value), timeout_ms); } @@ -298,9 +304,6 @@ class SocketConnection { } bool SendUint32(uint32_t value) { - if (!socket_.ok()) { - return false; - } int result = TEMP_FAILURE_RETRY(send(socket_, &value, sizeof(value), 0)); return result == sizeof(value); } @@ -315,9 +318,7 @@ class SocketConnection { return true; } - int Release() { return socket_.release(); } - - int socket() { return socket_.get(); } + int socket() { return socket_; } const ucred& cred() { return cred_; } @@ -388,34 +389,12 @@ class SocketConnection { return bytes_left == 0; } - unique_fd socket_; + int socket_; ucred cred_; + + DISALLOW_IMPLICIT_CONSTRUCTORS(SocketConnection); }; -static uint32_t SendControlMessage(const std::string& msg, const std::string& name, pid_t pid, - SocketConnection* socket, std::string* error) { - 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); - if (socket != nullptr) { - control_message->set_fd(socket->socket()); - } - - if (auto result = SendMessage(init_socket, property_msg); !result) { - *error = "Failed to send control message: " + result.error().message(); - return PROP_ERROR_HANDLE_CONTROL_MESSAGE; - } - - if (socket != nullptr) { - // We've successfully sent the fd to init, so release it here. - socket->Release(); - } - - return PROP_SUCCESS; -} - bool CheckControlPropertyPerms(const std::string& name, const std::string& value, const std::string& source_context, const ucred& cr) { // We check the legacy method first but these properties are dontaudit, so we only log an audit @@ -483,14 +462,15 @@ uint32_t CheckPermissions(const std::string& name, const std::string& value, // This returns one of the enum of PROP_SUCCESS or PROP_ERROR*. uint32_t HandlePropertySet(const std::string& name, const std::string& value, - const std::string& source_context, const ucred& cr, - SocketConnection* socket, std::string* error) { + const std::string& source_context, const ucred& cr, std::string* error) { if (auto ret = CheckPermissions(name, value, source_context, cr, error); ret != PROP_SUCCESS) { return ret; } if (StartsWith(name, "ctl.")) { - return SendControlMessage(name.c_str() + 4, value, cr.pid, socket, error); + return HandleControlMessage(name.c_str() + 4, value, cr.pid) + ? PROP_SUCCESS + : PROP_ERROR_HANDLE_CONTROL_MESSAGE; } // sys.powerctl is a special property that is used to make the device reboot. We want to log @@ -521,20 +501,6 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value, return PropertySet(name, value, error); } -uint32_t InitPropertySet(const std::string& name, const std::string& value) { - uint32_t result = 0; - ucred cr = {.pid = 1, .uid = 0, .gid = 0}; - std::string error; - result = HandlePropertySet(name, value, kInitContext.c_str(), cr, nullptr, &error); - if (result != PROP_SUCCESS) { - LOG(ERROR) << "Init cannot set '" << name << "' to '" << value << "': " << error; - } - - return result; -} - -uint32_t (*property_set)(const std::string& name, const std::string& value) = InitPropertySet; - static void handle_property_set_fd() { static constexpr uint32_t kDefaultSocketTimeout = 2000; /* ms */ @@ -583,8 +549,7 @@ static void handle_property_set_fd() { const auto& cr = socket.cred(); std::string error; - uint32_t result = - HandlePropertySet(prop_name, prop_value, source_context, cr, nullptr, &error); + uint32_t result = HandlePropertySet(prop_name, prop_value, source_context, cr, &error); if (result != PROP_SUCCESS) { LOG(ERROR) << "Unable to set property '" << prop_name << "' from uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid << ": " << error; @@ -612,12 +577,11 @@ static void handle_property_set_fd() { const auto& cr = socket.cred(); std::string error; - uint32_t result = HandlePropertySet(name, value, source_context, cr, &socket, &error); + uint32_t result = HandlePropertySet(name, value, source_context, cr, &error); if (result != PROP_SUCCESS) { LOG(ERROR) << "Unable to set property '" << name << "' from uid:" << cr.uid << " gid:" << cr.gid << " pid:" << cr.pid << ": " << error; } - socket.SendUint32(result); break; } @@ -800,6 +764,7 @@ void load_persist_props(void) { for (const auto& persistent_property_record : persistent_properties.properties()) { property_set(persistent_property_record.name(), persistent_property_record.value()); } + persistent_properties_loaded = true; property_set("ro.persistent_properties.ready", "true"); } @@ -1020,37 +985,21 @@ void CreateSerializedPropertyInfo() { selinux_android_restorecon(kPropertyInfosPath, 0); } -static void PropertyServiceThread() { - while (true) { - handle_property_set_fd(); - } -} - -void StartPropertyService(int* epoll_socket) { +void StartPropertyService(Epoll* epoll) { property_set("ro.property_service.version", "2"); - int sockets[2]; - 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]; - init_socket = sockets[1]; - - if (auto result = CreateSocket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC, false, 0666, 0, 0, - {})) { + if (auto result = CreateSocket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, + false, 0666, 0, 0, {})) { property_set_fd = *result; } else { - LOG(FATAL) << "start_property_service socket creation failed: " << result.error(); + PLOG(FATAL) << "start_property_service socket creation failed: " << result.error(); } listen(property_set_fd, 8); - std::thread{PropertyServiceThread}.detach(); - - property_set = [](const std::string& key, const std::string& value) -> uint32_t { - android::base::SetProperty(key, value); - return 0; - }; + if (auto result = epoll->RegisterHandler(property_set_fd, handle_property_set_fd); !result) { + PLOG(FATAL) << result.error(); + } } } // namespace init diff --git a/init/property_service.h b/init/property_service.h index 2dc92a5e9..7f9f84422 100644 --- a/init/property_service.h +++ b/init/property_service.h @@ -25,16 +25,17 @@ namespace android { namespace init { -static constexpr const char kRestoreconProperty[] = "selinux.restorecon_recursive"; - bool CanReadProperty(const std::string& source_context, const std::string& name); extern uint32_t (*property_set)(const std::string& name, const std::string& value); +uint32_t HandlePropertySet(const std::string& name, const std::string& value, + const std::string& source_context, const ucred& cr, std::string* error); + void property_init(); void property_load_boot_defaults(bool load_debug_prop); void load_persist_props(); -void StartPropertyService(int* epoll_socket); +void StartPropertyService(Epoll* epoll); } // namespace init } // namespace android diff --git a/init/property_service.proto b/init/property_service.proto deleted file mode 100644 index f7115269e..000000000 --- a/init/property_service.proto +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (C) 2019 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. - */ - -syntax = "proto2"; -option optimize_for = LITE_RUNTIME; - -message PropertyMessage { - message ControlMessage { - optional string msg = 1; - optional string name = 2; - optional int32 pid = 3; - optional int32 fd = 4; - } - - message ChangedMessage { - optional string name = 1; - optional string value = 2; - } - - oneof msg { - ControlMessage control_message = 1; - ChangedMessage changed_message = 2; - }; -} diff --git a/init/proto_utils.h b/init/proto_utils.h deleted file mode 100644 index 93a7d57a8..000000000 --- a/init/proto_utils.h +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright (C) 2019 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 - -#include - -#include "result.h" - -namespace android { -namespace init { - -constexpr size_t kBufferSize = 4096; - -inline Result ReadMessage(int socket) { - char buffer[kBufferSize] = {}; - auto result = TEMP_FAILURE_RETRY(recv(socket, buffer, sizeof(buffer), 0)); - if (result == 0) { - return Error(); - } else if (result < 0) { - return ErrnoError(); - } - return std::string(buffer, result); -} - -template -Result SendMessage(int socket, const T& message) { - std::string message_string; - if (!message.SerializeToString(&message_string)) { - return Error() << "Unable to serialize message"; - } - - if (message_string.size() > kBufferSize) { - return Error() << "Serialized message too long to send"; - } - - if (auto result = - TEMP_FAILURE_RETRY(send(socket, message_string.c_str(), message_string.size(), 0)); - result != static_cast(message_string.size())) { - return ErrnoError() << "send() failed to send message contents"; - } - return {}; -} - -} // namespace init -} // namespace android diff --git a/init/subcontext.cpp b/init/subcontext.cpp index ec93b5837..00f91d830 100644 --- a/init/subcontext.cpp +++ b/init/subcontext.cpp @@ -18,17 +18,16 @@ #include #include +#include #include #include #include -#include #include #include #include "action.h" #include "builtins.h" -#include "proto_utils.h" #include "util.h" #if defined(__ANDROID__) @@ -60,6 +59,45 @@ const char* const paths_and_secontexts[2][2] = { namespace { +constexpr size_t kBufferSize = 4096; + +Result ReadMessage(int socket) { + char buffer[kBufferSize] = {}; + auto result = TEMP_FAILURE_RETRY(recv(socket, buffer, sizeof(buffer), 0)); + if (result == 0) { + return Error(); + } else if (result < 0) { + return ErrnoError(); + } + return std::string(buffer, result); +} + +template +Result SendMessage(int socket, const T& message) { + std::string message_string; + if (!message.SerializeToString(&message_string)) { + return Error() << "Unable to serialize message"; + } + + if (message_string.size() > kBufferSize) { + return Error() << "Serialized message too long to send"; + } + + if (auto result = + TEMP_FAILURE_RETRY(send(socket, message_string.c_str(), message_string.size(), 0)); + result != static_cast(message_string.size())) { + return ErrnoError() << "send() failed to send message contents"; + } + return {}; +} + +std::vector> properties_to_set; + +uint32_t SubcontextPropertySet(const std::string& name, const std::string& value) { + properties_to_set.emplace_back(name, value); + return 0; +} + class SubcontextProcess { public: SubcontextProcess(const BuiltinFunctionMap* function_map, std::string context, int init_fd) @@ -93,6 +131,14 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec result = RunBuiltinFunction(map_result->function, args, context_); } + for (const auto& [name, value] : properties_to_set) { + auto property = reply->add_properties_to_set(); + property->set_name(name); + property->set_value(value); + } + + properties_to_set.clear(); + if (result) { reply->set_success(true); } else { @@ -178,10 +224,7 @@ int SubcontextMain(int argc, char** argv, const BuiltinFunctionMap* function_map SelabelInitialize(); - property_set = [](const std::string& key, const std::string& value) -> uint32_t { - android::base::SetProperty(key, value); - return 0; - }; + property_set = SubcontextPropertySet; auto subcontext_process = SubcontextProcess(function_map, context, init_fd); subcontext_process.MainLoop(); @@ -268,6 +311,15 @@ Result Subcontext::Execute(const std::vector& args) { return subcontext_reply.error(); } + for (const auto& property : subcontext_reply->properties_to_set()) { + ucred cr = {.pid = pid_, .uid = 0, .gid = 0}; + std::string error; + if (HandlePropertySet(property.name(), property.value(), context_, cr, &error) != 0) { + LOG(ERROR) << "Subcontext init could not set '" << property.name() << "' to '" + << property.value() << "': " << error; + } + } + if (subcontext_reply->reply_case() == SubcontextReply::kFailure) { auto& failure = subcontext_reply->failure(); return ResultError(failure.error_string(), failure.error_errno()); diff --git a/init/subcontext.proto b/init/subcontext.proto index e68115e0e..c31f4fb68 100644 --- a/init/subcontext.proto +++ b/init/subcontext.proto @@ -38,4 +38,10 @@ message SubcontextReply { Failure failure = 2; ExpandArgsReply expand_args_reply = 3; } + + message PropertyToSet { + optional string name = 1; + optional string value = 2; + } + repeated PropertyToSet properties_to_set = 4; } \ No newline at end of file