From 41043673b711d9f442ef23d11a67b073914e78f8 Mon Sep 17 00:00:00 2001
From: Odin Ugedal <odin@uged.al>
Date: Tue, 10 Aug 2021 21:24:38 +0100
Subject: [PATCH] libct/cg/sd/v1: Fix unnecessary freeze/thaw

This fixes the behavior intended to avoid freezing containers/control
groups without it being necessary. This is important for end users of
libcontainer who rely on the behavior of no freeze.

The previous implementation would always get error trying to get
DevicePolicy from the Unit via dbus, since the Unit interface doesn't
contain DevicePolicy.

Signed-off-by: Odin Ugedal <odin@uged.al>
---
 libcontainer/cgroups/systemd/common.go       | 12 ++++++++++--
 libcontainer/cgroups/systemd/systemd_test.go | 17 +++++++++++++++++
 libcontainer/cgroups/systemd/v1.go           | 20 +++++++++++++-------
 3 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go
index 1f3fc9638c2..98ccc51655c 100644
--- a/libcontainer/cgroups/systemd/common.go
+++ b/libcontainer/cgroups/systemd/common.go
@@ -311,6 +311,14 @@ func getUnitName(c *configs.Cgroup) string {
 	return c.Name
 }
 
+// This code should be in sync with getUnitName.
+func getUnitType(unitName string) string {
+	if strings.HasSuffix(unitName, ".slice") {
+		return "Slice"
+	}
+	return "Scope"
+}
+
 // isDbusError returns true if the error is a specific dbus error.
 func isDbusError(err error, name string) bool {
 	if err != nil {
@@ -389,10 +397,10 @@ func resetFailedUnit(cm *dbusConnManager, name string) {
 	}
 }
 
-func getUnitProperty(cm *dbusConnManager, unitName string, propertyName string) (*systemdDbus.Property, error) {
+func getUnitTypeProperty(cm *dbusConnManager, unitName string, unitType string, propertyName string) (*systemdDbus.Property, error) {
 	var prop *systemdDbus.Property
 	err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) (Err error) {
-		prop, Err = c.GetUnitPropertyContext(context.TODO(), unitName, propertyName)
+		prop, Err = c.GetUnitTypePropertyContext(context.TODO(), unitName, unitType, propertyName)
 		return Err
 	})
 	return prop, err
diff --git a/libcontainer/cgroups/systemd/systemd_test.go b/libcontainer/cgroups/systemd/systemd_test.go
index debcb8fd292..6976c4ca604 100644
--- a/libcontainer/cgroups/systemd/systemd_test.go
+++ b/libcontainer/cgroups/systemd/systemd_test.go
@@ -40,6 +40,23 @@ func TestSystemdVersion(t *testing.T) {
 	}
 }
 
+func TestValidUnitTypes(t *testing.T) {
+	testCases := []struct {
+		unitName         string
+		expectedUnitType string
+	}{
+		{"system.slice", "Slice"},
+		{"kubepods.slice", "Slice"},
+		{"testing-container:ab.scope", "Scope"},
+	}
+	for _, sdTest := range testCases {
+		unitType := getUnitType(sdTest.unitName)
+		if unitType != sdTest.expectedUnitType {
+			t.Errorf("getUnitType(%s); want %q; got %q", sdTest.unitName, sdTest.expectedUnitType, unitType)
+		}
+	}
+}
+
 func newManager(config *configs.Cgroup) cgroups.Manager {
 	if cgroups.IsCgroup2UnifiedMode() {
 		return NewUnifiedManager(config, "", false)
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
index 1a8e1e3c6c1..d1ea622b41a 100644
--- a/libcontainer/cgroups/systemd/v1.go
+++ b/libcontainer/cgroups/systemd/v1.go
@@ -6,6 +6,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"sync"
 
@@ -353,15 +354,20 @@ func (m *legacyManager) freezeBeforeSet(unitName string, r *configs.Resources) (
 		// a non-existent unit returns default properties,
 		// and settings in (2) are the defaults.
 		//
-		// Do not return errors from getUnitProperty, as they alone
+		// Do not return errors from getUnitTypeProperty, as they alone
 		// should not prevent Set from working.
-		devPolicy, e := getUnitProperty(m.dbus, unitName, "DevicePolicy")
+
+		unitType := getUnitType(unitName)
+
+		devPolicy, e := getUnitTypeProperty(m.dbus, unitName, unitType, "DevicePolicy")
 		if e == nil && devPolicy.Value == dbus.MakeVariant("auto") {
-			devAllow, e := getUnitProperty(m.dbus, unitName, "DeviceAllow")
-			if e == nil && devAllow.Value == dbus.MakeVariant([]deviceAllowEntry{}) {
-				needsFreeze = false
-				needsThaw = false
-				return
+			devAllow, e := getUnitTypeProperty(m.dbus, unitName, unitType, "DeviceAllow")
+			if e == nil {
+				if rv := reflect.ValueOf(devAllow.Value.Value()); rv.Kind() == reflect.Slice && rv.Len() == 0 {
+					needsFreeze = false
+					needsThaw = false
+					return
+				}
 			}
 		}
 	}