libprocessgroup: Prevent SetProcessProfiles from using cached fd
Because we cache file descriptors associated with cgroup "tasks" file it
should not be used with SetProcessProfiles API which operates on entire
processes rather than tasks. Change SetProcessProfiles API to prevent
cache fd usage, modify ExecuteForProcess to not attempt to use cached
fd. Also fix unconditional calls to EnableResourceCaching from
ExecuteForTask which should be called only when SetTaskProfiles is used
with use_fd_cache set to true.
Bug: 149524788
Change-Id: I880efaf8217a4dd7ccfbb4fb167b2295cefc057a
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h
index 0b38b6b..4aa439a 100644
--- a/libprocessgroup/include/processgroup/processgroup.h
+++ b/libprocessgroup/include/processgroup/processgroup.h
@@ -30,8 +30,7 @@
bool CgroupGetAttributePathForTask(const std::string& attr_name, int tid, std::string* path);
bool SetTaskProfiles(int tid, const std::vector<std::string>& profiles, bool use_fd_cache = false);
-bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector<std::string>& profiles,
- bool use_fd_cache = false);
+bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector<std::string>& profiles);
#ifndef __ANDROID_VNDK__
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 6272664..d669ebe 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -115,9 +115,8 @@
TaskProfiles::GetInstance().DropResourceCaching();
}
-bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector<std::string>& profiles,
- bool use_fd_cache) {
- return TaskProfiles::GetInstance().SetProcessProfiles(uid, pid, profiles, use_fd_cache);
+bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector<std::string>& profiles) {
+ return TaskProfiles::GetInstance().SetProcessProfiles(uid, pid, profiles);
}
bool SetTaskProfiles(int tid, const std::vector<std::string>& profiles, bool use_fd_cache) {
diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 72f01af..4af4589 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -201,22 +201,6 @@
}
bool SetCgroupAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
- std::lock_guard<std::mutex> lock(fd_mutex_);
- if (IsFdValid()) {
- // fd is cached, reuse it
- if (!AddTidToCgroup(pid, fd_)) {
- LOG(ERROR) << "Failed to add task into cgroup";
- return false;
- }
- return true;
- }
-
- if (fd_ == FDS_INACCESSIBLE) {
- // no permissions to access the file, ignore
- return true;
- }
-
- // this is app-dependent path and fd is not cached or cached fd can't be used
std::string procs_path = controller()->GetProcsFilePath(path_, uid, pid);
unique_fd tmp_fd(TEMP_FAILURE_RETRY(open(procs_path.c_str(), O_WRONLY | O_CLOEXEC)));
if (tmp_fd < 0) {
@@ -270,7 +254,6 @@
bool ApplyProfileAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
for (const auto& profile : profiles_) {
- profile->EnableResourceCaching();
if (!profile->ExecuteForProcess(uid, pid)) {
PLOG(WARNING) << "ExecuteForProcess failed for aggregate profile";
}
@@ -280,7 +263,6 @@
bool ApplyProfileAction::ExecuteForTask(int tid) const {
for (const auto& profile : profiles_) {
- profile->EnableResourceCaching();
if (!profile->ExecuteForTask(tid)) {
PLOG(WARNING) << "ExecuteForTask failed for aggregate profile";
}
@@ -288,6 +270,18 @@
return true;
}
+void ApplyProfileAction::EnableResourceCaching() {
+ for (const auto& profile : profiles_) {
+ profile->EnableResourceCaching();
+ }
+}
+
+void ApplyProfileAction::DropResourceCaching() {
+ for (const auto& profile : profiles_) {
+ profile->DropResourceCaching();
+ }
+}
+
void TaskProfile::MoveTo(TaskProfile* profile) {
profile->elements_ = std::move(elements_);
profile->res_cached_ = res_cached_;
@@ -527,13 +521,10 @@
}
bool TaskProfiles::SetProcessProfiles(uid_t uid, pid_t pid,
- const std::vector<std::string>& profiles, bool use_fd_cache) {
+ const std::vector<std::string>& profiles) {
for (const auto& name : profiles) {
TaskProfile* profile = GetProfile(name);
if (profile != nullptr) {
- if (use_fd_cache) {
- profile->EnableResourceCaching();
- }
if (!profile->ExecuteForProcess(uid, pid)) {
PLOG(WARNING) << "Failed to apply " << name << " process profile";
}
diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h
index a64ca50..28bc00c 100644
--- a/libprocessgroup/task_profiles.h
+++ b/libprocessgroup/task_profiles.h
@@ -163,6 +163,8 @@
virtual bool ExecuteForProcess(uid_t uid, pid_t pid) const;
virtual bool ExecuteForTask(int tid) const;
+ virtual void EnableResourceCaching();
+ virtual void DropResourceCaching();
private:
std::vector<std::shared_ptr<TaskProfile>> profiles_;
@@ -176,8 +178,7 @@
TaskProfile* GetProfile(const std::string& name) const;
const ProfileAttribute* GetAttribute(const std::string& name) const;
void DropResourceCaching() const;
- bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector<std::string>& profiles,
- bool use_fd_cache);
+ bool SetProcessProfiles(uid_t uid, pid_t pid, const std::vector<std::string>& profiles);
bool SetTaskProfiles(int tid, const std::vector<std::string>& profiles, bool use_fd_cache);
private: