From 8cacb6105ca3b35161403874cb8ec26061cf9e71 Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Wed, 12 Apr 2023 01:24:23 +0000 Subject: [PATCH] libprocessgroup: implement task profile validity checks Provide profile validity check functions for cases when user wants to check whether a profile can be successfully applied before actually applying it. Add test cases to cover new APIs. Also add a wrapper function for framework code to call it. Bug: 277233783 Test: atest task_profiles_test Test: manually verify freezer with outdated cgroup configuration Signed-off-by: Suren Baghdasaryan Signed-off-by: Li Li Change-Id: Iefb321dead27adbe67721972f164efea213c06cb --- .../include/processgroup/processgroup.h | 4 + libprocessgroup/processgroup.cpp | 10 ++ libprocessgroup/task_profiles.cpp | 121 ++++++++++++++++++ libprocessgroup/task_profiles.h | 20 ++- libprocessgroup/task_profiles_test.cpp | 50 ++++++++ 5 files changed, 202 insertions(+), 3 deletions(-) diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h index 48bc0b7f3..dbaeb9397 100644 --- a/libprocessgroup/include/processgroup/processgroup.h +++ b/libprocessgroup/include/processgroup/processgroup.h @@ -96,6 +96,10 @@ void removeAllEmptyProcessGroups(void); // Returns false in case of error, true in case of success bool getAttributePathForTask(const std::string& attr_name, int tid, std::string* path); +// Check if a profile can be applied without failing. +// Returns true if it can be applied without failing, false otherwise +bool isProfileValidForProcess(const std::string& profile_name, int uid, int pid); + #endif // __ANDROID_VNDK__ __END_DECLS diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index e2942602a..1f2904000 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -656,3 +656,13 @@ bool setProcessGroupLimit(uid_t, int pid, int64_t limit_in_bytes) { bool getAttributePathForTask(const std::string& attr_name, int tid, std::string* path) { return CgroupGetAttributePathForTask(attr_name, tid, path); } + +bool isProfileValidForProcess(const std::string& profile_name, int uid, int pid) { + const TaskProfile* tp = TaskProfiles::GetInstance().GetProfile(profile_name); + + if (tp == nullptr) { + return false; + } + + return tp->IsValidForProcess(uid, pid); +} \ No newline at end of file diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp index 17318289f..44dba2a16 100644 --- a/libprocessgroup/task_profiles.cpp +++ b/libprocessgroup/task_profiles.cpp @@ -259,6 +259,31 @@ bool SetAttributeAction::ExecuteForUID(uid_t uid) const { return true; } +bool SetAttributeAction::IsValidForProcess(uid_t, pid_t pid) const { + return IsValidForTask(pid); +} + +bool SetAttributeAction::IsValidForTask(int tid) const { + std::string path; + + if (!attribute_->GetPathForTask(tid, &path)) { + return false; + } + + if (!access(path.c_str(), W_OK)) { + // operation will succeed + return true; + } + + if (!access(path.c_str(), F_OK)) { + // file exists but not writable + return false; + } + + // file does not exist, ignore if optional + return optional_; +} + SetCgroupAction::SetCgroupAction(const CgroupController& c, const std::string& p) : controller_(c), path_(p) { FdCacheHelper::Init(controller_.GetTasksFilePath(path_), fd_[ProfileAction::RCT_TASK]); @@ -396,6 +421,39 @@ void SetCgroupAction::DropResourceCaching(ResourceCacheType cache_type) { FdCacheHelper::Drop(fd_[cache_type]); } +bool SetCgroupAction::IsValidForProcess(uid_t uid, pid_t pid) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_PROCESS])) { + return true; + } + + if (fd_[ProfileAction::RCT_PROCESS] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + std::string procs_path = controller()->GetProcsFilePath(path_, uid, pid); + return access(procs_path.c_str(), W_OK) == 0; +} + +bool SetCgroupAction::IsValidForTask(int) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_TASK])) { + return true; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_APP_DEPENDENT) { + // application-dependent path can't be used with tid + return false; + } + + std::string tasks_path = controller()->GetTasksFilePath(path_); + return access(tasks_path.c_str(), W_OK) == 0; +} + WriteFileAction::WriteFileAction(const std::string& task_path, const std::string& proc_path, const std::string& value, bool logfailures) : task_path_(task_path), proc_path_(proc_path), value_(value), logfailures_(logfailures) { @@ -532,6 +590,37 @@ void WriteFileAction::DropResourceCaching(ResourceCacheType cache_type) { FdCacheHelper::Drop(fd_[cache_type]); } +bool WriteFileAction::IsValidForProcess(uid_t, pid_t) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_PROCESS])) { + return true; + } + + if (fd_[ProfileAction::RCT_PROCESS] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + return access(proc_path_.empty() ? task_path_.c_str() : proc_path_.c_str(), W_OK) == 0; +} + +bool WriteFileAction::IsValidForTask(int) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_TASK])) { + return true; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_APP_DEPENDENT) { + // application-dependent path can't be used with tid + return false; + } + + return access(task_path_.c_str(), W_OK) == 0; +} + bool ApplyProfileAction::ExecuteForProcess(uid_t uid, pid_t pid) const { for (const auto& profile : profiles_) { profile->ExecuteForProcess(uid, pid); @@ -558,6 +647,24 @@ void ApplyProfileAction::DropResourceCaching(ResourceCacheType cache_type) { } } +bool ApplyProfileAction::IsValidForProcess(uid_t uid, pid_t pid) const { + for (const auto& profile : profiles_) { + if (!profile->IsValidForProcess(uid, pid)) { + return false; + } + } + return true; +} + +bool ApplyProfileAction::IsValidForTask(int tid) const { + for (const auto& profile : profiles_) { + if (!profile->IsValidForTask(tid)) { + return false; + } + } + return true; +} + void TaskProfile::MoveTo(TaskProfile* profile) { profile->elements_ = std::move(elements_); profile->res_cached_ = res_cached_; @@ -620,6 +727,20 @@ void TaskProfile::DropResourceCaching(ProfileAction::ResourceCacheType cache_typ res_cached_ = false; } +bool TaskProfile::IsValidForProcess(uid_t uid, pid_t pid) const { + for (const auto& element : elements_) { + if (!element->IsValidForProcess(uid, pid)) return false; + } + return true; +} + +bool TaskProfile::IsValidForTask(int tid) const { + for (const auto& element : elements_) { + if (!element->IsValidForTask(tid)) return false; + } + return true; +} + void TaskProfiles::DropResourceCaching(ProfileAction::ResourceCacheType cache_type) const { for (auto& iter : profiles_) { iter.second->DropResourceCaching(cache_type); diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h index a8ecb873d..a62c5b0a9 100644 --- a/libprocessgroup/task_profiles.h +++ b/libprocessgroup/task_profiles.h @@ -72,12 +72,14 @@ class ProfileAction { virtual const char* Name() const = 0; // Default implementations will fail - virtual bool ExecuteForProcess(uid_t, pid_t) const { return false; }; - virtual bool ExecuteForTask(int) const { return false; }; - virtual bool ExecuteForUID(uid_t) const { return false; }; + virtual bool ExecuteForProcess(uid_t, pid_t) const { return false; } + virtual bool ExecuteForTask(int) const { return false; } + virtual bool ExecuteForUID(uid_t) const { return false; } virtual void EnableResourceCaching(ResourceCacheType) {} virtual void DropResourceCaching(ResourceCacheType) {} + virtual bool IsValidForProcess(uid_t uid, pid_t pid) const { return false; } + virtual bool IsValidForTask(int tid) const { return false; } protected: enum CacheUseResult { SUCCESS, FAIL, UNUSED }; @@ -103,6 +105,8 @@ class SetTimerSlackAction : public ProfileAction { const char* Name() const override { return "SetTimerSlack"; } bool ExecuteForTask(int tid) const override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override { return true; } + bool IsValidForTask(int tid) const override { return true; } private: unsigned long slack_; @@ -120,6 +124,8 @@ class SetAttributeAction : public ProfileAction { bool ExecuteForProcess(uid_t uid, pid_t pid) const override; bool ExecuteForTask(int tid) const override; bool ExecuteForUID(uid_t uid) const override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; private: const IProfileAttribute* attribute_; @@ -137,6 +143,8 @@ class SetCgroupAction : public ProfileAction { bool ExecuteForTask(int tid) const override; void EnableResourceCaching(ResourceCacheType cache_type) override; void DropResourceCaching(ResourceCacheType cache_type) override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; const CgroupController* controller() const { return &controller_; } @@ -161,6 +169,8 @@ class WriteFileAction : public ProfileAction { bool ExecuteForTask(int tid) const override; void EnableResourceCaching(ResourceCacheType cache_type) override; void DropResourceCaching(ResourceCacheType cache_type) override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; private: std::string task_path_, proc_path_, value_; @@ -186,6 +196,8 @@ class TaskProfile { bool ExecuteForUID(uid_t uid) const; void EnableResourceCaching(ProfileAction::ResourceCacheType cache_type); void DropResourceCaching(ProfileAction::ResourceCacheType cache_type); + bool IsValidForProcess(uid_t uid, pid_t pid) const; + bool IsValidForTask(int tid) const; private: const std::string name_; @@ -204,6 +216,8 @@ class ApplyProfileAction : public ProfileAction { bool ExecuteForTask(int tid) const override; void EnableResourceCaching(ProfileAction::ResourceCacheType cache_type) override; void DropResourceCaching(ProfileAction::ResourceCacheType cache_type) override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; private: std::vector> profiles_; diff --git a/libprocessgroup/task_profiles_test.cpp b/libprocessgroup/task_profiles_test.cpp index 6a5b48bf3..eadbe7697 100644 --- a/libprocessgroup/task_profiles_test.cpp +++ b/libprocessgroup/task_profiles_test.cpp @@ -175,6 +175,32 @@ TEST_P(SetAttributeFixture, SetAttribute) { } } +class TaskProfileFixture : public TestWithParam { + public: + ~TaskProfileFixture() = default; +}; + +TEST_P(TaskProfileFixture, TaskProfile) { + // Treehugger runs host tests inside a container without cgroupv2 support. + if (!IsCgroupV2MountedRw()) { + GTEST_SKIP(); + return; + } + const TestParam params = GetParam(); + ProfileAttributeMock pa(params.attr_name); + // Test simple profile with one action + std::shared_ptr tp = std::make_shared("test_profile"); + tp->Add(std::make_unique(&pa, params.attr_value, params.optional_attr)); + EXPECT_EQ(tp->IsValidForProcess(getuid(), getpid()), params.result); + EXPECT_EQ(tp->IsValidForTask(getpid()), params.result); + // Test aggregate profile + TaskProfile tp2("meta_profile"); + std::vector> profiles = {tp}; + tp2.Add(std::make_unique(profiles)); + EXPECT_EQ(tp2.IsValidForProcess(getuid(), getpid()), params.result); + EXPECT_EQ(tp2.IsValidForTask(getpid()), params.result); +} + // Test the four combinations of optional_attr {false, true} and cgroup attribute { does not exist, // exists }. INSTANTIATE_TEST_SUITE_P( @@ -215,4 +241,28 @@ INSTANTIATE_TEST_SUITE_P( .log_prefix = "Failed to write", .log_suffix = geteuid() == 0 ? "Invalid argument" : "Permission denied"})); +// Test TaskProfile IsValid calls. +INSTANTIATE_TEST_SUITE_P( + TaskProfileTestSuite, TaskProfileFixture, + Values( + // Test operating on non-existing cgroup attribute fails. + TestParam{.attr_name = "no-such-attribute", + .attr_value = ".", + .optional_attr = false, + .result = false}, + // Test operating on optional non-existing cgroup attribute succeeds. + TestParam{.attr_name = "no-such-attribute", + .attr_value = ".", + .optional_attr = true, + .result = true}, + // Test operating on existing cgroup attribute succeeds. + TestParam{.attr_name = "cgroup.procs", + .attr_value = ".", + .optional_attr = false, + .result = true}, + // Test operating on optional existing cgroup attribute succeeds. + TestParam{.attr_name = "cgroup.procs", + .attr_value = ".", + .optional_attr = true, + .result = true})); } // namespace