Fix a race condition in Service::Start()

The SetTaskProfiles() call modifies cgroup attributes. Modifying cgroup
attributes can only succeed after the cgroups and cgroup attributes have
been created. Hence this patch that makes the child process wait until
the parent has finished creating cgroups and activating cgroup
controllers.

Bug: 213617178
Test: Without this patch the migration to the v2 hierarchy does not work reliably. With this patch applied, the migration to the v2 hierarchy works reliably.
Change-Id: I80a7c0a35453d8fd89ed798d077086aa8ba9ea17
Signed-off-by: Bart Van Assche <bvanassche@google.com>
This commit is contained in:
Bart Van Assche 2022-01-26 20:52:52 +00:00
parent efc9366188
commit ee36ba39f9
1 changed files with 25 additions and 0 deletions

View File

@ -397,6 +397,14 @@ Result<void> Service::ExecStart() {
return {}; return {};
} }
static void ClosePipe(const std::array<int, 2>* pipe) {
for (const auto fd : *pipe) {
if (fd >= 0) {
close(fd);
}
}
}
Result<void> Service::Start() { Result<void> Service::Start() {
auto reboot_on_failure = make_scope_guard([this] { auto reboot_on_failure = make_scope_guard([this] {
if (on_failure_reboot_target_) { if (on_failure_reboot_target_) {
@ -428,6 +436,12 @@ Result<void> Service::Start() {
return {}; return {};
} }
std::unique_ptr<std::array<int, 2>, decltype(&ClosePipe)> pipefd(new std::array<int, 2>{-1, -1},
ClosePipe);
if (pipe(pipefd->data()) < 0) {
return ErrnoError() << "pipe()";
}
bool needs_console = (flags_ & SVC_CONSOLE); bool needs_console = (flags_ & SVC_CONSOLE);
if (needs_console) { if (needs_console) {
if (proc_attr_.console.empty()) { if (proc_attr_.console.empty()) {
@ -532,6 +546,13 @@ Result<void> Service::Start() {
LOG(ERROR) << "failed to write pid to files: " << result.error(); LOG(ERROR) << "failed to write pid to files: " << result.error();
} }
// Wait until the cgroups have been created and until the cgroup controllers have been
// activated.
if (std::byte byte; read((*pipefd)[0], &byte, 1) < 0) {
PLOG(ERROR) << "failed to read from notification channel";
}
pipefd.reset();
if (task_profiles_.size() > 0 && !SetTaskProfiles(getpid(), task_profiles_)) { if (task_profiles_.size() > 0 && !SetTaskProfiles(getpid(), task_profiles_)) {
LOG(ERROR) << "failed to set task profiles"; LOG(ERROR) << "failed to set task profiles";
} }
@ -618,6 +639,10 @@ Result<void> Service::Start() {
LmkdRegister(name_, proc_attr_.uid, pid_, oom_score_adjust_); LmkdRegister(name_, proc_attr_.uid, pid_, oom_score_adjust_);
} }
if (write((*pipefd)[1], "", 1) < 0) {
return ErrnoError() << "sending notification failed";
}
NotifyStateChange("running"); NotifyStateChange("running");
reboot_on_failure.Disable(); reboot_on_failure.Disable();
return {}; return {};