Revert "init: Fix a race condition in KillProcessGroup()"

This reverts commit 15e5ecdcd7.

Reason for revert: breaks console support.
Bug: 213617178
Bug: 258754901
Change-Id: Iffe213e2cd295461a427621f2b84933f1bebd39f

init/service.cpp

@@ -16,7 +16,6 @@
 
 #include "service.h"
 
-#include <errno.h>
 #include <fcntl.h>
 #include <inttypes.h>
 #include <linux/securebits.h>
@@ -533,6 +532,7 @@ void Service::RunService(const std::vector<Descriptor>& descriptors, Interproces
     if (!byte.ok()) {
         LOG(ERROR) << name_ << ": failed to read from notification channel: " << byte.error();
     }
+    fifo.Close();
     if (!*byte) {
         LOG(FATAL) << "Service '" << name_  << "' failed to start due to a fatal error";
         _exit(EXIT_FAILURE);
@@ -556,12 +556,6 @@ void Service::RunService(const std::vector<Descriptor>& descriptors, Interproces
     // priority. Aborts on failure.
     SetProcessAttributesAndCaps();
 
-    // If SetProcessAttributes() called setsid(), report this to the parent.
-    if (RequiresConsole(proc_attr_)) {
-        fifo.Write(2);
-    }
-    fifo.Close();
-
     if (!ExpandArgsAndExecv(args_, sigstop_)) {
         PLOG(ERROR) << "cannot execv('" << args_[0]
                     << "'). See the 'Debugging init' section of init's README.md for tips";
@@ -662,8 +656,11 @@ Result<void> Service::Start() {
 
     if (pid == 0) {
         umask(077);
+        fifo.CloseWriteFd();
         RunService(descriptors, std::move(fifo));
         _exit(127);
+    } else {
+        fifo.CloseReadFd();
     }
 
     if (pid < 0) {
@@ -720,31 +717,6 @@ Result<void> Service::Start() {
         return Error() << "Sending cgroups activated notification failed: " << result.error();
     }
 
-    // Call setpgid() from the parent process to make sure that this call has
-    // finished before the parent process calls kill(-pgid, ...).
-    if (proc_attr_.console.empty()) {
-        if (setpgid(pid, pid) < 0) {
-            switch (errno) {
-                case EACCES:   // Child has already performed execve().
-                case ESRCH:    // Child process no longer exists.
-                    break;
-                default:
-                    PLOG(ERROR) << "setpgid() from parent failed";
-            }
-        }
-    } else {
-        // The Read() call below will return an error if the child is killed.
-        if (Result<uint8_t> result = fifo.Read(); !result.ok() || *result != 2) {
-            if (!result.ok()) {
-                return Error() << "Waiting for setsid() failed: " << result.error();
-            } else {
-                return Error() << "Waiting for setsid() failed: " << *result << " <> 2";
-            }
-        }
-    }
-
-    fifo.Close();
-
     NotifyStateChange("running");
     reboot_on_failure.Disable();
     return {};

init/service.h

@@ -155,7 +155,7 @@ class Service {
     void ResetFlagsForStart();
     Result<void> CheckConsole();
     void ConfigureMemcg();
-    void RunService(const std::vector<Descriptor>& descriptors, InterprocessFifo fifo);
+    void RunService(const std::vector<Descriptor>& descriptors, InterprocessFifo cgroups_activated);
     void SetMountNamespace();
     static unsigned long next_start_order_;
     static bool is_exec_service_running_;

init/service_utils.cpp

@@ -240,15 +240,11 @@ Result<void> SetProcessAttributes(const ProcessAttributes& attr) {
         }
     }
 
-    if (RequiresConsole(attr)) {
+    if (!attr.console.empty()) {
         setsid();
         OpenConsole(attr.console);
     } else {
-        // Without PID namespaces, this call duplicates the setpgid() call from
-        // the parent process. With PID namespaces, this setpgid() call sets the
-        // process group ID for a child of the init process in the PID
-        // namespace.
-        if (setpgid(0, 0) == -1) {
+        if (setpgid(0, getpid()) == -1) {
             return ErrnoError() << "setpgid failed";
         }
         SetupStdio(attr.stdio_to_kmsg);

init/service_utils.h

@@ -89,11 +89,6 @@ struct ProcessAttributes {
     int priority;
     bool stdio_to_kmsg;
 };
-
-inline bool RequiresConsole(const ProcessAttributes& attr) {
-    return !attr.console.empty();
-}
-
 Result<void> SetProcessAttributes(const ProcessAttributes& attr);
 
 Result<void> WritePidToFiles(std::vector<std::string>* files);