init: simplify async restorecon
In the future, property service may run in its own thread or process,
which means that PropertyChildReap() needs to be refactored to not run
as part of the init signal handler.
The new method spawns a new thread that handles the queue of paths
that require restorecon. It then communicates back to property service
via android::base::SetProperty(). Property service distinguishes the
thread from other callers of SetProperty() by checking the pid in the
credentials for the socket connection, thus avoiding dependencies on
the rest of init.
The new method also drops the genericness, since restorecon is the
only function that we should ever need to run asynchronously
Test: async restorecon works, including with queued requests
Change-Id: I2ca00459969e77b1820776dac23d0a0d974e330b
diff --git a/init/property_service.cpp b/init/property_service.cpp
index bf3b317..81adbe4 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -41,7 +41,9 @@
#include <map>
#include <memory>
+#include <mutex>
#include <queue>
+#include <thread>
#include <vector>
#include <android-base/chrono_utils.h>
@@ -83,6 +85,8 @@
namespace android {
namespace init {
+static constexpr const char kRestoreconProperty[] = "selinux.restorecon_recursive";
+
static bool persistent_properties_loaded = false;
static int property_set_fd = -1;
@@ -187,88 +191,51 @@
return PROP_SUCCESS;
}
-typedef int (*PropertyAsyncFunc)(const std::string&, const std::string&);
+class AsyncRestorecon {
+ public:
+ void TriggerRestorecon(const std::string& path) {
+ auto guard = std::lock_guard{mutex_};
+ paths_.emplace(path);
-struct PropertyChildInfo {
- pid_t pid;
- PropertyAsyncFunc func;
- std::string name;
- std::string value;
+ if (!thread_started_) {
+ thread_started_ = true;
+ std::thread{&AsyncRestorecon::ThreadFunction, this}.detach();
+ }
+ }
+
+ private:
+ void ThreadFunction() {
+ auto lock = std::unique_lock{mutex_};
+
+ while (!paths_.empty()) {
+ auto path = paths_.front();
+ paths_.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);
+ lock.lock();
+ }
+
+ thread_started_ = false;
+ }
+
+ std::mutex mutex_;
+ std::queue<std::string> paths_;
+ bool thread_started_ = false;
};
-static std::queue<PropertyChildInfo> property_children;
-
-static void PropertyChildLaunch() {
- auto& info = property_children.front();
- pid_t pid = fork();
- if (pid < 0) {
- LOG(ERROR) << "Failed to fork for property_set_async";
- while (!property_children.empty()) {
- property_children.pop();
- }
- return;
- }
- if (pid != 0) {
- info.pid = pid;
- } else {
- if (info.func(info.name, info.value) != 0) {
- LOG(ERROR) << "property_set_async(\"" << info.name << "\", \"" << info.value
- << "\") failed";
- }
- _exit(0);
- }
-}
-
-bool PropertyChildReap(pid_t pid) {
- if (property_children.empty()) {
- return false;
- }
- auto& info = property_children.front();
- if (info.pid != pid) {
- return false;
- }
- std::string error;
- if (PropertySet(info.name, info.value, &error) != PROP_SUCCESS) {
- LOG(ERROR) << "Failed to set async property " << info.name << " to " << info.value << ": "
- << error;
- }
- property_children.pop();
- if (!property_children.empty()) {
- PropertyChildLaunch();
- }
- return true;
-}
-
-static uint32_t PropertySetAsync(const std::string& name, const std::string& value,
- PropertyAsyncFunc func, std::string* error) {
- if (value.empty()) {
- return PropertySet(name, value, error);
- }
-
- PropertyChildInfo info;
- info.func = func;
- info.name = name;
- info.value = value;
- property_children.push(info);
- if (property_children.size() == 1) {
- PropertyChildLaunch();
- }
- return PROP_SUCCESS;
-}
-
-static int RestoreconRecursiveAsync(const std::string& name, const std::string& value) {
- return selinux_android_restorecon(value.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE);
-}
-
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 == "selinux.restorecon_recursive") {
- LOG(ERROR) << "InitPropertySet: Do not set selinux.restorecon_recursive from init; use the "
- "restorecon builtin directly";
+ if (name == kRestoreconProperty) {
+ LOG(ERROR) << "InitPropertySet: Do not set '" << kRestoreconProperty
+ << "' from init; use the restorecon builtin directly";
return PROP_ERROR_INVALID_NAME;
}
@@ -506,8 +473,14 @@
<< process_log_string;
}
- if (name == "selinux.restorecon_recursive") {
- return PropertySetAsync(name, value, RestoreconRecursiveAsync, error);
+ // If a process other than init is writing a non-empty value, it means that process is
+ // requesting that init performs a restorecon operation on the path specified by '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);
+ return PROP_SUCCESS;
}
return PropertySet(name, value, error);
@@ -661,7 +634,7 @@
}
if (StartsWith(key, "ctl.") || key == "sys.powerctl"s ||
- key == "selinux.restorecon_recursive"s) {
+ std::string{key} == kRestoreconProperty) {
LOG(ERROR) << "Ignoring disallowed property '" << key
<< "' with special meaning in prop file '" << filename << "'";
continue;
diff --git a/init/property_service.h b/init/property_service.h
index 207c03b..7f9f844 100644
--- a/init/property_service.h
+++ b/init/property_service.h
@@ -32,8 +32,6 @@
uint32_t HandlePropertySet(const std::string& name, const std::string& value,
const std::string& source_context, const ucred& cr, std::string* error);
-extern bool PropertyChildReap(pid_t pid);
-
void property_init();
void property_load_boot_defaults(bool load_debug_prop);
void load_persist_props();
diff --git a/init/sigchld_handler.cpp b/init/sigchld_handler.cpp
index 0b03324..987b2f9 100644
--- a/init/sigchld_handler.cpp
+++ b/init/sigchld_handler.cpp
@@ -29,7 +29,6 @@
#include <android-base/stringprintf.h>
#include "init.h"
-#include "property_service.h"
#include "service.h"
using android::base::StringPrintf;
@@ -61,9 +60,7 @@
std::string wait_string;
Service* service = nullptr;
- if (PropertyChildReap(pid)) {
- name = "Async property child";
- } else if (SubcontextChildReap(pid)) {
+ if (SubcontextChildReap(pid)) {
name = "Subcontext";
} else {
service = ServiceList::GetInstance().FindService(pid, &Service::pid);
