init: always kill oneshot services' process groups.

~2007 a change was added that would allow oneshot services to
daemonize by not killing their process group.  This was a hack at the
time, and should certainly not be needed now.  I've resisted removing
the behavior however, as it hadn't caused any issues.

Recently, it was detected that the cgroups that these processes belong
to, would exist forever and therefore leak memory.  Instead of simply
removing the cgroups when empty, this provides a good opportunity to
do the right thing and fix this behavior once and for all.

The new (correct) behavior only happens for devices with vendor images
built for Android R or later.  Init will log a warning to dmesg when
it detects this difference in behavior has occurred.

Bug: 144545923
Test: boot CF/Coral and see no difference in behavior.
Test: boot CF with a service that daemonizes and see the warning.
Change-Id: I333a2e25a541ec0114ac50ab8ae7f1ea3f055447
This commit is contained in:
Tom Cherry 2019-11-19 14:19:40 -08:00
parent 7d16aedc47
commit d89ed132a0
4 changed files with 43 additions and 12 deletions

View File

@ -42,9 +42,11 @@
#if defined(__ANDROID__)
#include <ApexProperties.sysprop.h>
#include <android/api-level.h>
#include "mount_namespace.h"
#include "property_service.h"
#include "selinux.h"
#else
#include "host_init_stubs.h"
#endif
@ -182,7 +184,7 @@ void Service::NotifyStateChange(const std::string& new_state) const {
}
}
void Service::KillProcessGroup(int signal) {
void Service::KillProcessGroup(int signal, bool report_oneshot) {
// If we've already seen a successful result from killProcessGroup*(), then we have removed
// the cgroup already and calling these functions a second time will simply result in an error.
// This is true regardless of which signal was sent.
@ -190,11 +192,20 @@ void Service::KillProcessGroup(int signal) {
if (!process_cgroup_empty_) {
LOG(INFO) << "Sending signal " << signal << " to service '" << name_ << "' (pid " << pid_
<< ") process group...";
int max_processes = 0;
int r;
if (signal == SIGTERM) {
r = killProcessGroupOnce(proc_attr_.uid, pid_, signal);
r = killProcessGroupOnce(proc_attr_.uid, pid_, signal, &max_processes);
} else {
r = killProcessGroup(proc_attr_.uid, pid_, signal);
r = killProcessGroup(proc_attr_.uid, pid_, signal, &max_processes);
}
if (report_oneshot && max_processes > 0) {
LOG(WARNING)
<< "Killed " << max_processes
<< " additional processes from a oneshot process group for service '" << name_
<< "'. This is new behavior, previously child processes would not be killed in "
"this case.";
}
if (r == 0) process_cgroup_empty_ = true;
@ -244,7 +255,16 @@ void Service::SetProcessAttributesAndCaps() {
void Service::Reap(const siginfo_t& siginfo) {
if (!(flags_ & SVC_ONESHOT) || (flags_ & SVC_RESTART)) {
KillProcessGroup(SIGKILL);
KillProcessGroup(SIGKILL, false);
} else {
// Legacy behavior from ~2007 until Android R: this else branch did not exist and we did not
// kill the process group in this case.
if (SelinuxGetVendorAndroidVersion() >= __ANDROID_API_R__) {
// The new behavior in Android R is to kill these process groups in all cases. The
// 'true' parameter instructions KillProcessGroup() to report a warning message where it
// detects a difference in behavior has occurred.
KillProcessGroup(SIGKILL, true);
}
}
// Remove any socket resources we may have created.

View File

@ -132,7 +132,7 @@ class Service {
private:
void NotifyStateChange(const std::string& new_state) const;
void StopOrReset(int how);
void KillProcessGroup(int signal);
void KillProcessGroup(int signal, bool report_oneshot = false);
void SetProcessAttributesAndCaps();
static unsigned long next_start_order_;

View File

@ -47,11 +47,14 @@ void DropTaskProfilesResourceCaching();
// Return 0 and removes the cgroup if there are no longer any processes in it.
// Returns -1 in the case of an error occurring or if there are processes still running
// even after retrying for up to 200ms.
int killProcessGroup(uid_t uid, int initialPid, int signal);
// If max_processes is not nullptr, it returns the maximum number of processes seen in the cgroup
// during the killing process. Note that this can be 0 if all processes from the process group have
// already been terminated.
int killProcessGroup(uid_t uid, int initialPid, int signal, int* max_processes = nullptr);
// Returns the same as killProcessGroup(), however it does not retry, which means
// that it only returns 0 in the case that the cgroup exists and it contains no processes.
int killProcessGroupOnce(uid_t uid, int initialPid, int signal);
int killProcessGroupOnce(uid_t uid, int initialPid, int signal, int* max_processes = nullptr);
int createProcessGroup(uid_t uid, int initialPid, bool memControl = false);

View File

@ -301,7 +301,8 @@ static int DoKillProcessGroupOnce(const char* cgroup, uid_t uid, int initialPid,
return feof(fd.get()) ? processes : -1;
}
static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries) {
static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries,
int* max_processes) {
std::string cpuacct_path;
std::string memory_path;
@ -316,9 +317,16 @@ static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries)
std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();
if (max_processes != nullptr) {
*max_processes = 0;
}
int retry = retries;
int processes;
while ((processes = DoKillProcessGroupOnce(cgroup, uid, initialPid, signal)) > 0) {
if (max_processes != nullptr && processes > *max_processes) {
*max_processes = processes;
}
LOG(VERBOSE) << "Killed " << processes << " processes for processgroup " << initialPid;
if (retry > 0) {
std::this_thread::sleep_for(5ms);
@ -359,12 +367,12 @@ static int KillProcessGroup(uid_t uid, int initialPid, int signal, int retries)
}
}
int killProcessGroup(uid_t uid, int initialPid, int signal) {
return KillProcessGroup(uid, initialPid, signal, 40 /*retries*/);
int killProcessGroup(uid_t uid, int initialPid, int signal, int* max_processes) {
return KillProcessGroup(uid, initialPid, signal, 40 /*retries*/, max_processes);
}
int killProcessGroupOnce(uid_t uid, int initialPid, int signal) {
return KillProcessGroup(uid, initialPid, signal, 0 /*retries*/);
int killProcessGroupOnce(uid_t uid, int initialPid, int signal, int* max_processes) {
return KillProcessGroup(uid, initialPid, signal, 0 /*retries*/, max_processes);
}
int createProcessGroup(uid_t uid, int initialPid, bool memControl) {