From 53f79e6861302ed58118b9a5624e97183b4dc3e0 Mon Sep 17 00:00:00 2001
From: mtk16036 <squall.ko@mediatek.com>
Date: Fri, 31 May 2019 19:05:22 +0800
Subject: [PATCH] race condition in libprocessgroup

while enable fdsan (file descriptor sanitizer),
fdsan report use-after-close error after boot complete (sedom).

Because, in SetCgroupAction::EnableResourceCaching() currently has a data race against all the
use fd_ functions like SetCgroupAction::ExecuteForProcess(uid_t uid, pid_t pid) etc.

ThreadA                                     | ThreadB
-------------------------------------------------------------------------------------------------
in SetCgroupAction::EnableResourceCaching() | in SetCgroupAction::ExecuteForProcess(...)
-------------------------------------------------------------------------------------------------
                                            | in SetCgroupAction::AddTidToCgroup(int tid, int fd)
-------------------------------------------------------------------------------------------------
fd_ = std::move(fd); /*modified fd_ value*/ |
-------------------------------------------------------------------------------------------------
                                            | write(fd)  /* crash here, fd is closed by ThreadA*/
-------------------------------------------------------------------------------------------------

So, add mutex lock to protect fd_ data race.

Bug: 134120826
Test: auto test, run the adb reboot test 100 times and no fdsan error report on libprocessgroup
Change-Id: Iccf2f705e030f79324f1164509e715dc5be825de
---
 libprocessgroup/task_profiles.cpp | 3 +++
 libprocessgroup/task_profiles.h   | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 40d8d902c..edc316a8d 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -150,6 +150,7 @@ SetCgroupAction::SetCgroupAction(const CgroupController& c, const std::string& p
 }
 
 void SetCgroupAction::EnableResourceCaching() {
+    std::lock_guard<std::mutex> lock(fd_mutex_);
     if (fd_ != FDS_NOT_CACHED) {
         return;
     }
@@ -191,6 +192,7 @@ 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_)) {
@@ -221,6 +223,7 @@ bool SetCgroupAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
 }
 
 bool SetCgroupAction::ExecuteForTask(int tid) const {
+    std::lock_guard<std::mutex> lock(fd_mutex_);
     if (IsFdValid()) {
         // fd is cached, reuse it
         if (!AddTidToCgroup(tid, fd_)) {
diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h
index 445647dea..77bac2d94 100644
--- a/libprocessgroup/task_profiles.h
+++ b/libprocessgroup/task_profiles.h
@@ -19,6 +19,7 @@
 #include <sys/cdefs.h>
 #include <sys/types.h>
 #include <map>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -127,6 +128,7 @@ class SetCgroupAction : public ProfileAction {
     CgroupController controller_;
     std::string path_;
     android::base::unique_fd fd_;
+    mutable std::mutex fd_mutex_;
 
     static bool IsAppDependentPath(const std::string& path);
     static bool AddTidToCgroup(int tid, int fd);