From 1ed0a5661b5b9f2d9938478bad97140bbb272dc6 Mon Sep 17 00:00:00 2001
From: Jan Vorlicek <janvorli@microsoft.com>
Date: Wed, 18 Dec 2019 17:07:36 +0100
Subject: [PATCH] Fix named cgroup handling in docker (#980)

While named cgroups work fine outside of docker container, they weren't
working when created and used inside of a docker container. The problem
was caused by the fact that the hierarchy root extracted from
/proc/self/mountinfo and the cgroup path extracted from /proc/self/cgroup
are not equal for named groups. They just share the same prefix.
The cgroups handling code was not epxecting this case and ended up building
the final cgroup path incorrectly (including the common part of the path).
This change fixes it by checking for matching prefix of the paths instead
of comparing the whole paths.
---
 src/coreclr/src/gc/unix/cgroup.cpp      | 28 +++++++++++++++++++++++++---
 src/coreclr/src/pal/src/misc/cgroup.cpp | 27 ++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/src/gc/unix/cgroup.cpp b/src/coreclr/src/gc/unix/cgroup.cpp
index 0515f5e..df808c4 100644
--- a/src/coreclr/src/gc/unix/cgroup.cpp
+++ b/src/coreclr/src/gc/unix/cgroup.cpp
@@ -153,6 +153,7 @@ private:
         char *hierarchy_mount = nullptr;
         char *hierarchy_root = nullptr;
         char *cgroup_path_relative_to_mount = nullptr;
+        size_t common_path_prefix_len;
 
         FindHierarchyMount(is_subsystem, &hierarchy_mount, &hierarchy_root);
         if (hierarchy_mount == nullptr || hierarchy_root == nullptr)
@@ -168,9 +169,30 @@ private:
 
         strcpy(cgroup_path, hierarchy_mount);
         // For a host cgroup, we need to append the relative path.
-        // In a docker container, the root and relative path are the same and we don't need to append.
-        if (strcmp(hierarchy_root, cgroup_path_relative_to_mount) != 0)
-            strcat(cgroup_path, cgroup_path_relative_to_mount);
+        // The root and cgroup path can share a common prefix of the path that should not be appended.
+        // Example 1 (docker):
+        // hierarchy_mount:               /sys/fs/cgroup/cpu
+        // hierarchy_root:                /docker/87ee2de57e51bc75175a4d2e81b71d162811b179d549d6601ed70b58cad83578
+        // cgroup_path_relative_to_mount: /docker/87ee2de57e51bc75175a4d2e81b71d162811b179d549d6601ed70b58cad83578/my_named_cgroup
+        // append do the cgroup_path:     /my_named_cgroup
+        // final cgroup_path:             /sys/fs/cgroup/cpu/my_named_cgroup
+        //
+        // Example 2 (out of docker)
+        // hierarchy_mount:               /sys/fs/cgroup/cpu
+        // hierarchy_root:                /
+        // cgroup_path_relative_to_mount: /my_named_cgroup
+        // append do the cgroup_path:     /my_named_cgroup
+        // final cgroup_path:             /sys/fs/cgroup/cpu/my_named_cgroup
+        common_path_prefix_len = strlen(hierarchy_root);
+        if ((common_path_prefix_len == 1) || strncmp(hierarchy_root, cgroup_path_relative_to_mount, common_path_prefix_len) != 0)
+        {
+            common_path_prefix_len = 0;
+        }
+
+        assert(cgroup_path_relative_to_mount[common_path_prefix_len] == '/');
+
+        strcat(cgroup_path, cgroup_path_relative_to_mount + common_path_prefix_len);
+
 
     done:
         free(hierarchy_mount);
diff --git a/src/coreclr/src/pal/src/misc/cgroup.cpp b/src/coreclr/src/pal/src/misc/cgroup.cpp
index 53df734..6016036 100644
--- a/src/coreclr/src/pal/src/misc/cgroup.cpp
+++ b/src/coreclr/src/pal/src/misc/cgroup.cpp
@@ -141,6 +141,7 @@ private:
         char *hierarchy_root = nullptr;
         char *cgroup_path_relative_to_mount = nullptr;
         size_t len;
+        size_t common_path_prefix_len;
 
         FindHierarchyMount(is_subsystem, &hierarchy_mount, &hierarchy_root);
         if (hierarchy_mount == nullptr || hierarchy_root == nullptr)
@@ -158,9 +159,29 @@ private:
 
         strcpy_s(cgroup_path, len+1, hierarchy_mount);
         // For a host cgroup, we need to append the relative path.
-        // In a docker container, the root and relative path are the same and we don't need to append.
-        if (strcmp(hierarchy_root, cgroup_path_relative_to_mount) != 0)
-            strcat_s(cgroup_path, len+1, cgroup_path_relative_to_mount);
+        // The root and cgroup path can share a common prefix of the path that should not be appended.
+        // Example 1 (docker):
+        // hierarchy_mount:               /sys/fs/cgroup/cpu
+        // hierarchy_root:                /docker/87ee2de57e51bc75175a4d2e81b71d162811b179d549d6601ed70b58cad83578
+        // cgroup_path_relative_to_mount: /docker/87ee2de57e51bc75175a4d2e81b71d162811b179d549d6601ed70b58cad83578/my_named_cgroup
+        // append do the cgroup_path:     /my_named_cgroup
+        // final cgroup_path:             /sys/fs/cgroup/cpu/my_named_cgroup
+        //
+        // Example 2 (out of docker)
+        // hierarchy_mount:               /sys/fs/cgroup/cpu
+        // hierarchy_root:                /
+        // cgroup_path_relative_to_mount: /my_named_cgroup
+        // append do the cgroup_path:     /my_named_cgroup
+        // final cgroup_path:             /sys/fs/cgroup/cpu/my_named_cgroup
+        common_path_prefix_len = strlen(hierarchy_root);
+        if ((common_path_prefix_len == 1) || strncmp(hierarchy_root, cgroup_path_relative_to_mount, common_path_prefix_len) != 0)
+        {
+            common_path_prefix_len = 0;
+        }
+
+        _ASSERTE(cgroup_path_relative_to_mount[common_path_prefix_len] == '/');
+
+        strcat_s(cgroup_path, len+1, cgroup_path_relative_to_mount + common_path_prefix_len);
 
     done:
         PAL_free(hierarchy_mount);
-- 
2.7.4