From f3478b7516e02ab341324ae1361ea9b019ae4f4e Mon Sep 17 00:00:00 2001
From: Fabricio Voznika <fvoznika@google.com>
Date: Thu, 13 May 2021 14:41:38 -0700
Subject: Fix problem with grouped cgroups

cgroup controllers can be grouped together (e.g. cpu,cpuacct) and
that was confusing Cgroup.Install() into thinking that a cgroup
directory was created by the caller, when it had being created by
another controller that is grouped together.

PiperOrigin-RevId: 373661336
---
 runsc/cgroup/cgroup.go      | 35 ++++++++++++++++++++++-------------
 runsc/cgroup/cgroup_test.go |  4 ++--
 2 files changed, 24 insertions(+), 15 deletions(-)

(limited to 'runsc/cgroup')

diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go
index 335e46499..66a6a0f68 100644
--- a/runsc/cgroup/cgroup.go
+++ b/runsc/cgroup/cgroup.go
@@ -342,25 +342,31 @@ func new(pid, cgroupsPath string) (*Cgroup, error) {
 // already exists, it means that the caller has already provided a
 // pre-configured cgroups, and 'res' is ignored.
 func (c *Cgroup) Install(res *specs.LinuxResources) error {
-	log.Debugf("Creating cgroup %q", c.Name)
+	log.Debugf("Installing cgroup path %q", c.Name)
 
-	// The Cleanup object cleans up partially created cgroups when an error occurs.
-	// Errors occuring during cleanup itself are ignored.
+	// Clean up partially created cgroups on error. Errors during cleanup itself
+	// are ignored.
 	clean := cleanup.Make(func() { _ = c.Uninstall() })
 	defer clean.Clean()
 
-	for key, ctrlr := range controllers {
+	// Controllers can be symlinks to a group of controllers (e.g. cpu,cpuacct).
+	// So first check what directories need to be created. Otherwise, when
+	// the directory for one of the controllers in a group is created, it will
+	// make it seem like the directory already existed and it's not owned by the
+	// other controllers in the group.
+	var missing []string
+	for key := range controllers {
 		path := c.MakePath(key)
-		if _, err := os.Stat(path); err == nil {
-			// If cgroup has already been created; it has been setup by caller. Don't
-			// make any changes to configuration, just join when sandbox/gofer starts.
-			log.Debugf("Using pre-created cgroup %q", path)
-			continue
+		if _, err := os.Stat(path); err != nil {
+			missing = append(missing, key)
+		} else {
+			log.Debugf("Using pre-created cgroup %q: %q", key, path)
 		}
-
-		// Mark that cgroup resources are owned by me.
-		c.Own[key] = true
-
+	}
+	for _, key := range missing {
+		ctrlr := controllers[key]
+		path := c.MakePath(key)
+		log.Debugf("Creating cgroup %q: %q", key, path)
 		if err := os.MkdirAll(path, 0755); err != nil {
 			if ctrlr.optional() && errors.Is(err, unix.EROFS) {
 				if err := ctrlr.skip(res); err != nil {
@@ -371,6 +377,9 @@ func (c *Cgroup) Install(res *specs.LinuxResources) error {
 			}
 			return err
 		}
+
+		// Only set controllers that were created by me.
+		c.Own[key] = true
 		if err := ctrlr.set(res, path); err != nil {
 			return err
 		}
diff --git a/runsc/cgroup/cgroup_test.go b/runsc/cgroup/cgroup_test.go
index 02cadeef4..eba40621e 100644
--- a/runsc/cgroup/cgroup_test.go
+++ b/runsc/cgroup/cgroup_test.go
@@ -60,10 +60,10 @@ var dindMountinfo = `
 
 func TestUninstallEnoent(t *testing.T) {
 	c := Cgroup{
-		// set a non-existent name
+		// Use a non-existent name.
 		Name: "runsc-test-uninstall-656e6f656e740a",
+		Own:  make(map[string]bool),
 	}
-	c.Own = make(map[string]bool)
 	for key := range controllers {
 		c.Own[key] = true
 	}
-- 
cgit v1.2.3