Revert "Reland: "init: run property service in a thread""
This reverts commit 8efca4bbb378ff5bd3af06d8511ea75a7ed49f99.
Reason for revert: Still broken
Change-Id: I3b37b1b00ff4b19f2eec2d8bd72042463d47cee3
diff --git a/init/Android.bp b/init/Android.bp
index 7dfb828..285a6a4 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -128,7 +128,6 @@
"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 3573e34..e17e899 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<void> 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 e5d1036..18fb0c3 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -28,9 +28,6 @@
#include <sys/types.h>
#include <unistd.h>
-#define _REALLY_INCLUDE_SYS__SYSTEM_PROPERTIES_H_
-#include <sys/_system_properties.h>
-
#include <functional>
#include <map>
#include <memory>
@@ -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 char qemu[32];
static int signal_fd = -1;
-static int property_fd = -1;
static std::unique_ptr<Timer> waiting_for_prop(nullptr);
static std::string wait_prop_name;
@@ -619,44 +613,6 @@
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 @@
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 624a3d4..cfc28f1 100644
--- a/init/init.h
+++ b/init/init.h
@@ -31,6 +31,10 @@
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 c78b81b..3408ff3 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -42,7 +42,6 @@
#include <map>
#include <memory>
#include <mutex>
-#include <optional>
#include <queue>
#include <thread>
#include <vector>
@@ -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::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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 2dc92a5..7f9f844 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 f711526..0000000
--- 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 93a7d57..0000000
--- 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 <sys/socket.h>
-#include <unistd.h>
-
-#include <string>
-
-#include "result.h"
-
-namespace android {
-namespace init {
-
-constexpr size_t kBufferSize = 4096;
-
-inline Result<std::string> 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 <typename T>
-Result<void> 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<long>(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 ec93b58..00f91d8 100644
--- a/init/subcontext.cpp
+++ b/init/subcontext.cpp
@@ -18,17 +18,16 @@
#include <fcntl.h>
#include <poll.h>
+#include <sys/socket.h>
#include <unistd.h>
#include <android-base/file.h>
#include <android-base/logging.h>
-#include <android-base/properties.h>
#include <android-base/strings.h>
#include <selinux/android.h>
#include "action.h"
#include "builtins.h"
-#include "proto_utils.h"
#include "util.h"
#if defined(__ANDROID__)
@@ -60,6 +59,45 @@
namespace {
+constexpr size_t kBufferSize = 4096;
+
+Result<std::string> 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 <typename T>
+Result<void> 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<long>(message_string.size())) {
+ return ErrnoError() << "send() failed to send message contents";
+ }
+ return {};
+}
+
+std::vector<std::pair<std::string, std::string>> 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 @@
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 @@
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 @@
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 e68115e..c31f4fb 100644
--- a/init/subcontext.proto
+++ b/init/subcontext.proto
@@ -38,4 +38,10 @@
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