Merge changes I4c55790c,I14baaa7a
* changes:
init: Fix a race condition in KillProcessGroup()
init: Document that ReapOneProcess() does not modify 'pid'
diff --git a/init/service.cpp b/init/service.cpp
index 331cd88..caa9095 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -507,7 +507,7 @@
// Enters namespaces, sets environment variables, writes PID files and runs the service executable.
void Service::RunService(const std::vector<Descriptor>& descriptors,
- InterprocessFifo cgroups_activated) {
+ InterprocessFifo cgroups_activated, InterprocessFifo setsid_finished) {
if (auto result = EnterNamespaces(namespaces_, name_, mount_namespace_); !result.ok()) {
LOG(FATAL) << "Service '" << name_ << "' failed to set up namespaces: " << result.error();
}
@@ -557,6 +557,12 @@
// priority. Aborts on failure.
SetProcessAttributesAndCaps();
+ // If SetProcessAttributes() called setsid(), report this to the parent.
+ if (!proc_attr_.console.empty()) {
+ setsid_finished.Write(2);
+ }
+ setsid_finished.Close();
+
if (!ExpandArgsAndExecv(args_, sigstop_)) {
PLOG(ERROR) << "cannot execv('" << args_[0]
<< "'). See the 'Debugging init' section of init's README.md for tips";
@@ -598,7 +604,7 @@
return {};
}
- InterprocessFifo cgroups_activated;
+ InterprocessFifo cgroups_activated, setsid_finished;
if (Result<void> result = cgroups_activated.Initialize(); !result.ok()) {
return result;
@@ -608,6 +614,13 @@
return result;
}
+ // Only check proc_attr_.console after the CheckConsole() call.
+ if (!proc_attr_.console.empty()) {
+ if (Result<void> result = setsid_finished.Initialize(); !result.ok()) {
+ return result;
+ }
+ }
+
struct stat sb;
if (stat(args_[0].c_str(), &sb) == -1) {
flags_ |= SVC_DISABLED;
@@ -661,10 +674,12 @@
if (pid == 0) {
umask(077);
cgroups_activated.CloseWriteFd();
- RunService(descriptors, std::move(cgroups_activated));
+ setsid_finished.CloseReadFd();
+ RunService(descriptors, std::move(cgroups_activated), std::move(setsid_finished));
_exit(127);
} else {
cgroups_activated.CloseReadFd();
+ setsid_finished.CloseWriteFd();
}
if (pid < 0) {
@@ -721,6 +736,23 @@
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) == -1) {
+ return ErrnoError() << "setpgid failed";
+ }
+ } else {
+ // The Read() call below will return an error if the child is killed.
+ if (Result<uint8_t> result = setsid_finished.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";
+ }
+ }
+ }
+
NotifyStateChange("running");
reboot_on_failure.Disable();
return {};
diff --git a/init/service.h b/init/service.h
index b2c9909..10a0790 100644
--- a/init/service.h
+++ b/init/service.h
@@ -155,7 +155,8 @@
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 cgroups_activated,
+ InterprocessFifo setsid_finished);
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..56a80b5 100644
--- a/init/service_utils.cpp
+++ b/init/service_utils.cpp
@@ -244,7 +244,11 @@
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/sigchld_handler.cpp b/init/sigchld_handler.cpp
index 6a6050b..f8c501f 100644
--- a/init/sigchld_handler.cpp
+++ b/init/sigchld_handler.cpp
@@ -53,8 +53,13 @@
return 0;
}
- auto pid = siginfo.si_pid;
- if (pid == 0) return 0;
+ const pid_t pid = siginfo.si_pid;
+ if (pid == 0) {
+ DCHECK_EQ(siginfo.si_signo, 0);
+ return 0;
+ }
+
+ DCHECK_EQ(siginfo.si_signo, SIGCHLD);
// At this point we know we have a zombie pid, so we use this scopeguard to reap the pid
// whenever the function returns from this point forward.