From ee36ba39f9733b609bd5ebd5b6fa91c7522d994d Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bvanassche@google.com>
Date: Wed, 26 Jan 2022 20:52:52 +0000
Subject: [PATCH] Fix a race condition in Service::Start()

The SetTaskProfiles() call modifies cgroup attributes. Modifying cgroup
attributes can only succeed after the cgroups and cgroup attributes have
been created. Hence this patch that makes the child process wait until
the parent has finished creating cgroups and activating cgroup
controllers.

Bug: 213617178
Test: Without this patch the migration to the v2 hierarchy does not work reliably. With this patch applied, the migration to the v2 hierarchy works reliably.
Change-Id: I80a7c0a35453d8fd89ed798d077086aa8ba9ea17
Signed-off-by: Bart Van Assche <bvanassche@google.com>
---
 init/service.cpp | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/init/service.cpp b/init/service.cpp
index f7318cba4..f6dd9b9fa 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -397,6 +397,14 @@ Result<void> Service::ExecStart() {
     return {};
 }
 
+static void ClosePipe(const std::array<int, 2>* pipe) {
+    for (const auto fd : *pipe) {
+        if (fd >= 0) {
+            close(fd);
+        }
+    }
+}
+
 Result<void> Service::Start() {
     auto reboot_on_failure = make_scope_guard([this] {
         if (on_failure_reboot_target_) {
@@ -428,6 +436,12 @@ Result<void> Service::Start() {
         return {};
     }
 
+    std::unique_ptr<std::array<int, 2>, decltype(&ClosePipe)> pipefd(new std::array<int, 2>{-1, -1},
+                                                                     ClosePipe);
+    if (pipe(pipefd->data()) < 0) {
+        return ErrnoError() << "pipe()";
+    }
+
     bool needs_console = (flags_ & SVC_CONSOLE);
     if (needs_console) {
         if (proc_attr_.console.empty()) {
@@ -532,6 +546,13 @@ Result<void> Service::Start() {
             LOG(ERROR) << "failed to write pid to files: " << result.error();
         }
 
+        // Wait until the cgroups have been created and until the cgroup controllers have been
+        // activated.
+        if (std::byte byte; read((*pipefd)[0], &byte, 1) < 0) {
+            PLOG(ERROR) << "failed to read from notification channel";
+        }
+        pipefd.reset();
+
         if (task_profiles_.size() > 0 && !SetTaskProfiles(getpid(), task_profiles_)) {
             LOG(ERROR) << "failed to set task profiles";
         }
@@ -618,6 +639,10 @@ Result<void> Service::Start() {
         LmkdRegister(name_, proc_attr_.uid, pid_, oom_score_adjust_);
     }
 
+    if (write((*pipefd)[1], "", 1) < 0) {
+        return ErrnoError() << "sending notification failed";
+    }
+
     NotifyStateChange("running");
     reboot_on_failure.Disable();
     return {};