Commit 53f79e68 authored May 31, 2019 by mtk16036 Committed by Suren Baghdasaryan Jun 11, 2019

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

parent 1d7f3b4f

libprocessgroup/task_profiles.cpp

+3 −0

Original line number	Diff line number	Diff line
		@@ -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_)) {

libprocessgroup/task_profiles.h

+2 −0

Original line number	Diff line number	Diff line
		@@ -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);