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

This commit is contained in:
Bart Van Assche 2022-11-03 18:21:59 +00:00 committed by Gerrit Code Review
commit 8462ceef62
3 changed files with 5 additions and 42 deletions

View file

@ -507,7 +507,7 @@ void Service::ConfigureMemcg() {
// 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 setsid_finished) {
InterprocessFifo cgroups_activated) {
if (auto result = EnterNamespaces(namespaces_, name_, mount_namespace_); !result.ok()) {
LOG(FATAL) << "Service '" << name_ << "' failed to set up namespaces: " << result.error();
}
@ -557,12 +557,6 @@ void Service::RunService(const std::vector<Descriptor>& descriptors,
// 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";
@ -604,7 +598,7 @@ Result<void> Service::Start() {
return {};
}
InterprocessFifo cgroups_activated, setsid_finished;
InterprocessFifo cgroups_activated;
if (Result<void> result = cgroups_activated.Initialize(); !result.ok()) {
return result;
@ -614,13 +608,6 @@ Result<void> Service::Start() {
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;
@ -674,12 +661,10 @@ Result<void> Service::Start() {
if (pid == 0) {
umask(077);
cgroups_activated.CloseWriteFd();
setsid_finished.CloseReadFd();
RunService(descriptors, std::move(cgroups_activated), std::move(setsid_finished));
RunService(descriptors, std::move(cgroups_activated));
_exit(127);
} else {
cgroups_activated.CloseReadFd();
setsid_finished.CloseWriteFd();
}
if (pid < 0) {
@ -736,23 +721,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) == -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 {};

View file

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

View file

@ -244,11 +244,7 @@ Result<void> SetProcessAttributes(const ProcessAttributes& attr) {
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);