Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 911109c4 authored by Suren Baghdasaryan's avatar Suren Baghdasaryan
Browse files

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: default avatarSuren Baghdasaryan <surenb@google.com>
parent c7b92ea1

libprocessgroup/include/processgroup/processgroup.h

+1 −2
Original line number Diff line number Diff line
@@ -30,8 +30,7 @@ bool CgroupGetAttributePath(const std::string& attr_name, std::string* path); 
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__

libprocessgroup/processgroup.cpp

+2 −3
Original line number Diff line number Diff line
@@ -115,9 +115,8 @@ void DropTaskProfilesResourceCaching() { 
    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) {

libprocessgroup/task_profiles.cpp

+13 −22
Original line number Diff line number Diff line
@@ -201,22 +201,6 @@ bool SetCgroupAction::AddTidToCgroup(int tid, int fd) { 
}



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 SetCgroupAction::ExecuteForTask(int tid) const { 


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::ExecuteForProcess(uid_t uid, pid_t pid) const { 


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 @@ bool ApplyProfileAction::ExecuteForTask(int tid) const { 
    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 @@ const ProfileAttribute* TaskProfiles::GetAttribute(const std::string& name) cons 
}



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";

            }

libprocessgroup/task_profiles.h

+3 −2
Original line number Diff line number Diff line
@@ -163,6 +163,8 @@ class ApplyProfileAction : public ProfileAction { 


    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 @@ class TaskProfiles { 
    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: