From 4ce440f22ae89cd28abd20f4dcb6e40ddee2ab7d 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>
(cherry picked from commit 41043673b711d9f442ef23d11a67b073914e78f8)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 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 3506c8274..06c6c2e95 100644
--- a/libcontainer/cgroups/systemd/common.go
+++ b/libcontainer/cgroups/systemd/common.go
@@ -310,6 +310,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 {
@@ -388,10 +396,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 55685d055..0f9041fab 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 1a8e1e3c6..d1ea622b4 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
+				}
 			}
 		}
 	}