From 752e7a824946abb297afb59825d7003fed639a7f Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Sat, 22 May 2021 14:33:54 -0700
Subject: [PATCH] libct/cg/sd: fix SkipDevices for systemd

Commit 108ee85b8271e adds SkipDevices flag, which is used by kubernetes
to create cgroups for pods.

Unfortunately the above commit falls short, and systemd DevicePolicy and
DeviceAllow properties are still set, which requires kubernetes to set
"allow everything" rule.

This commit fixes this: if SkipDevices flag is set, we return
Device* properties to allow all devices.

Fixes: 108ee85b8271e
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/systemd/common.go | 34 ++++++++++++++++----------
 libcontainer/cgroups/systemd/v1.go     |  2 +-
 libcontainer/cgroups/systemd/v2.go     |  2 +-
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go
index 91c314e09..7dafbd406 100644
--- a/libcontainer/cgroups/systemd/common.go
+++ b/libcontainer/cgroups/systemd/common.go
@@ -158,14 +158,27 @@ func findDeviceGroup(ruleType devices.Type, ruleMajor int64) (string, error) {
 	return "", nil
 }
 
+// DeviceAllow is the dbus type "a(ss)" which means we need a struct
+// to represent it in Go.
+type deviceAllowEntry struct {
+	Path  string
+	Perms string
+}
+
+func allowAllDevices() []systemdDbus.Property {
+	// Setting mode to auto and removing all DeviceAllow rules
+	// results in allowing access to all devices.
+	return []systemdDbus.Property{
+		newProp("DevicePolicy", "auto"),
+		newProp("DeviceAllow", []deviceAllowEntry{}),
+	}
+}
+
 // generateDeviceProperties takes the configured device rules and generates a
 // corresponding set of systemd properties to configure the devices correctly.
-func generateDeviceProperties(rules []*devices.Rule) ([]systemdDbus.Property, error) {
-	// DeviceAllow is the type "a(ss)" which means we need a temporary struct
-	// to represent it in Go.
-	type deviceAllowEntry struct {
-		Path  string
-		Perms string
+func generateDeviceProperties(r *configs.Resources) ([]systemdDbus.Property, error) {
+	if r.SkipDevices {
+		return allowAllDevices(), nil
 	}
 
 	properties := []systemdDbus.Property{
@@ -177,7 +190,7 @@ func generateDeviceProperties(rules []*devices.Rule) ([]systemdDbus.Property, er
 
 	// Figure out the set of rules.
 	configEmu := &cgroupdevices.Emulator{}
-	for _, rule := range rules {
+	for _, rule := range r.Devices {
 		if err := configEmu.Apply(*rule); err != nil {
 			return nil, errors.Wrap(err, "apply rule for systemd")
 		}
@@ -189,12 +202,7 @@ func generateDeviceProperties(rules []*devices.Rule) ([]systemdDbus.Property, er
 	if configEmu.IsBlacklist() {
 		// However, if we're dealing with an allow-all rule then we can do it.
 		if configEmu.IsAllowAll() {
-			return []systemdDbus.Property{
-				// Run in white-list mode by setting to "auto" and removing all
-				// DeviceAllow rules.
-				newProp("DevicePolicy", "auto"),
-				newProp("DeviceAllow", []deviceAllowEntry{}),
-			}, nil
+			return allowAllDevices(), nil
 		}
 		logrus.Warn("systemd doesn't support blacklist device rules -- applying temporary deny-all rule")
 		return properties, nil
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
index 41de6e8b7..e735156c7 100644
--- a/libcontainer/cgroups/systemd/v1.go
+++ b/libcontainer/cgroups/systemd/v1.go
@@ -61,7 +61,7 @@ var legacySubsystems = []subsystem{
 func genV1ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]systemdDbus.Property, error) {
 	var properties []systemdDbus.Property
 
-	deviceProperties, err := generateDeviceProperties(r.Devices)
+	deviceProperties, err := generateDeviceProperties(r)
 	if err != nil {
 		return nil, err
 	}
diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
index 8abb0feb7..7be3da1d7 100644
--- a/libcontainer/cgroups/systemd/v2.go
+++ b/libcontainer/cgroups/systemd/v2.go
@@ -172,7 +172,7 @@ func genV2ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]syst
 	//       aren't the end of the world, but it is a bit concerning. However
 	//       it's unclear if systemd removes all eBPF programs attached when
 	//       doing SetUnitProperties...
-	deviceProperties, err := generateDeviceProperties(r.Devices)
+	deviceProperties, err := generateDeviceProperties(r)
 	if err != nil {
 		return nil, err
 	}