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

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 @@ 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;
@@ -187,88 +191,51 @@ static uint32_t PropertySet(const std::string& name, const std::string& value, s
     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 @@ uint32_t HandlePropertySet(const std::string& name, const std::string& value,
                   << 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 @@ static void LoadProperties(char* data, const char* filter, const char* filename,
             }
 
             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;

init/property_service.h

@@ -32,8 +32,6 @@ extern uint32_t (*property_set)(const std::string& name, const std::string& valu
 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();

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 @@ static bool ReapOneProcess() {
     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);