Merge "init: Fix a race condition in KillProcessGroup()"
diff --git a/init/service.cpp b/init/service.cpp
index c260c07..85ac2fc 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -16,6 +16,7 @@
#include "service.h"
+#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
#include <linux/securebits.h>
@@ -532,7 +533,6 @@
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,6 +556,12 @@
// 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";
@@ -656,11 +662,8 @@
if (pid == 0) {
umask(077);
- fifo.CloseWriteFd();
RunService(descriptors, std::move(fifo));
_exit(127);
- } else {
- fifo.CloseReadFd();
}
if (pid < 0) {
@@ -717,6 +720,31 @@
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 {};
diff --git a/init/service.h b/init/service.h
index b2c9909..2c2778d 100644
--- a/init/service.h
+++ b/init/service.h
@@ -155,7 +155,7 @@
void ResetFlagsForStart();
Result<void> CheckConsole();
void ConfigureMemcg();
- void RunService(const std::vector<Descriptor>& descriptors, InterprocessFifo cgroups_activated);
+ void RunService(const std::vector<Descriptor>& descriptors, InterprocessFifo fifo);
void SetMountNamespace();
static unsigned long next_start_order_;
static bool is_exec_service_running_;
diff --git a/init/service_utils.cpp b/init/service_utils.cpp
index a14969e..9585d05 100644
--- a/init/service_utils.cpp
+++ b/init/service_utils.cpp
@@ -240,11 +240,15 @@
}
}
- if (!attr.console.empty()) {
+ if (RequiresConsole(attr)) {
setsid();
OpenConsole(attr.console);
} else {
- if (setpgid(0, getpid()) == -1) {
+ // 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) {
return ErrnoError() << "setpgid failed";
}
SetupStdio(attr.stdio_to_kmsg);
diff --git a/init/service_utils.h b/init/service_utils.h
index 65a2012..c66f2b4 100644
--- a/init/service_utils.h
+++ b/init/service_utils.h
@@ -89,6 +89,11 @@
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);