Merge "libprocessgroup: restrict SetupCgroups to one-time usage and only by init"

3 files changed, 35 insertions, 24 deletions

diff --git a/libprocessgroup/cgroup_map.cpp b/libprocessgroup/cgroup_map.cpp
index b3b497f68..1e66fa4ee 100644
--- a/libprocessgroup/cgroup_map.cpp
+++ b/libprocessgroup/cgroup_map.cpp
@@ -227,13 +227,16 @@ static bool SetupCgroup(const CgroupDescriptor&) {
 
 #endif
 
+// WARNING: This function should be called only from SetupCgroups and only once.
+// It intentionally leaks an FD, so additional invocation will result in additional leak.
 static bool WriteRcFile(const std::map<std::string, CgroupDescriptor>& descriptors) {
-    std::string cgroup_rc_path = StringPrintf("%s/%s", CGROUPS_RC_DIR, CgroupMap::CGROUPS_RC_FILE);
-    unique_fd fd(TEMP_FAILURE_RETRY(open(cgroup_rc_path.c_str(),
-                                         O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
-                                         S_IRUSR | S_IRGRP | S_IROTH)));
+    // WARNING: We are intentionally leaking the FD to keep the file open forever.
+    // Let init keep the FD open to prevent file mappings from becoming invalid in
+    // case the file gets deleted somehow.
+    int fd = TEMP_FAILURE_RETRY(open(CGROUPS_RC_PATH, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC,
+                                     S_IRUSR | S_IRGRP | S_IROTH));
     if (fd < 0) {
-        PLOG(ERROR) << "open() failed for " << cgroup_rc_path;
+        PLOG(ERROR) << "open() failed for " << CGROUPS_RC_PATH;
         return false;
     }
 
@@ -242,14 +245,14 @@ static bool WriteRcFile(const std::map<std::string, CgroupDescriptor>& descripto
     fl.controller_count_ = descriptors.size();
     int ret = TEMP_FAILURE_RETRY(write(fd, &fl, sizeof(fl)));
     if (ret < 0) {
-        PLOG(ERROR) << "write() failed for " << cgroup_rc_path;
+        PLOG(ERROR) << "write() failed for " << CGROUPS_RC_PATH;
         return false;
     }
 
     for (const auto& [name, descriptor] : descriptors) {
         ret = TEMP_FAILURE_RETRY(write(fd, descriptor.controller(), sizeof(CgroupController)));
         if (ret < 0) {
-            PLOG(ERROR) << "write() failed for " << cgroup_rc_path;
+            PLOG(ERROR) << "write() failed for " << CGROUPS_RC_PATH;
             return false;
         }
     }
@@ -350,38 +353,37 @@ bool CgroupMap::LoadRcFile() {
         return true;
     }
 
-    std::string cgroup_rc_path = StringPrintf("%s/%s", CGROUPS_RC_DIR, CGROUPS_RC_FILE);
-    unique_fd fd(TEMP_FAILURE_RETRY(open(cgroup_rc_path.c_str(), O_RDONLY | O_CLOEXEC)));
+    unique_fd fd(TEMP_FAILURE_RETRY(open(CGROUPS_RC_PATH, O_RDONLY | O_CLOEXEC)));
     if (fd < 0) {
-        PLOG(ERROR) << "open() failed for " << cgroup_rc_path;
+        PLOG(ERROR) << "open() failed for " << CGROUPS_RC_PATH;
         return false;
     }
 
     if (fstat(fd, &sb) < 0) {
-        PLOG(ERROR) << "fstat() failed for " << cgroup_rc_path;
+        PLOG(ERROR) << "fstat() failed for " << CGROUPS_RC_PATH;
         return false;
     }
 
     size_t file_size = sb.st_size;
     if (file_size < sizeof(CgroupFile)) {
-        LOG(ERROR) << "Invalid file format " << cgroup_rc_path;
+        LOG(ERROR) << "Invalid file format " << CGROUPS_RC_PATH;
         return false;
     }
 
     CgroupFile* file_data = (CgroupFile*)mmap(nullptr, file_size, PROT_READ, MAP_SHARED, fd, 0);
     if (file_data == MAP_FAILED) {
-        PLOG(ERROR) << "Failed to mmap " << cgroup_rc_path;
+        PLOG(ERROR) << "Failed to mmap " << CGROUPS_RC_PATH;
         return false;
     }
 
     if (file_data->version_ != CgroupFile::FILE_CURR_VERSION) {
-        LOG(ERROR) << cgroup_rc_path << " file version mismatch";
+        LOG(ERROR) << CGROUPS_RC_PATH << " file version mismatch";
         munmap(file_data, file_size);
         return false;
     }
 
     if (file_size != sizeof(CgroupFile) + file_data->controller_count_ * sizeof(CgroupController)) {
-        LOG(ERROR) << cgroup_rc_path << " file has invalid size";
+        LOG(ERROR) << CGROUPS_RC_PATH << " file has invalid size";
         munmap(file_data, file_size);
         return false;
     }
@@ -412,6 +414,18 @@ void CgroupMap::Print() const {
 bool CgroupMap::SetupCgroups() {
     std::map<std::string, CgroupDescriptor> descriptors;
 
+    if (getpid() != 1) {
+        LOG(ERROR) << "Cgroup setup can be done only by init process";
+        return false;
+    }
+
+    // Make sure we do this only one time. No need for std::call_once because
+    // init is a single-threaded process
+    if (access(CGROUPS_RC_PATH, F_OK) == 0) {
+        LOG(WARNING) << "Attempt to call SetupCgroups more than once";
+        return true;
+    }
+
     // load cgroups.json file
     if (!ReadDescriptors(&descriptors)) {
         LOG(ERROR) << "Failed to load cgroup description file";
@@ -428,8 +442,8 @@ bool CgroupMap::SetupCgroups() {
     }
 
     // mkdir <CGROUPS_RC_DIR> 0711 system system
-    if (!Mkdir(CGROUPS_RC_DIR, 0711, "system", "system")) {
-        LOG(ERROR) << "Failed to create directory for <CGROUPS_RC_FILE> file";
+    if (!Mkdir(android::base::Dirname(CGROUPS_RC_PATH), 0711, "system", "system")) {
+        LOG(ERROR) << "Failed to create directory for " << CGROUPS_RC_PATH << " file";
         return false;
     }
 
@@ -438,13 +452,12 @@ bool CgroupMap::SetupCgroups() {
     // and limits infrormation shared with unprivileged processes
     // to the minimum subset of information from cgroups.json
     if (!WriteRcFile(descriptors)) {
-        LOG(ERROR) << "Failed to write " << CGROUPS_RC_FILE << " file";
+        LOG(ERROR) << "Failed to write " << CGROUPS_RC_PATH << " file";
         return false;
     }
 
-    std::string cgroup_rc_path = StringPrintf("%s/%s", CGROUPS_RC_DIR, CGROUPS_RC_FILE);
-    // chmod 0644 <cgroup_rc_path>
-    if (fchmodat(AT_FDCWD, cgroup_rc_path.c_str(), 0644, AT_SYMLINK_NOFOLLOW) < 0) {
+    // chmod 0644 <CGROUPS_RC_PATH>
+    if (fchmodat(AT_FDCWD, CGROUPS_RC_PATH, 0644, AT_SYMLINK_NOFOLLOW) < 0) {
         PLOG(ERROR) << "fchmodat() failed";
         return false;
     }
diff --git a/libprocessgroup/cgroup_map.h b/libprocessgroup/cgroup_map.h
index 1c355cd17..304ae155e 100644
--- a/libprocessgroup/cgroup_map.h
+++ b/libprocessgroup/cgroup_map.h
@@ -75,8 +75,6 @@ struct CgroupFile {
 
 class CgroupMap {
   public:
-    static constexpr const char* CGROUPS_RC_FILE = "cgroup.rc";
-
     // Selinux policy ensures only init process can successfully use this function
     static bool SetupCgroups();
 
diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h
index 6f973b832..46b86762a 100644
--- a/libprocessgroup/include/processgroup/processgroup.h
+++ b/libprocessgroup/include/processgroup/processgroup.h
@@ -24,7 +24,7 @@
 __BEGIN_DECLS
 
 static constexpr const char* CGROUPV2_CONTROLLER_NAME = "cgroup2";
-static constexpr const char* CGROUPS_RC_DIR = "/dev/cgroup_info";
+static constexpr const char* CGROUPS_RC_PATH = "/dev/cgroup_info/cgroup.rc";
 
 bool CgroupSetupCgroups();
 bool CgroupGetControllerPath(const std::string& cgroup_name, std::string* path);