libct/cg/sd: fix "SkipDevices" handling

1. The meaning of SkipDevices is what it is -- do not set any
   device-related options.

2. Reverts the part of commit 108ee85 which skipped the freeze
   when the SkipDevices is set. Apparently, the freeze is needed on
   update even if no Device* properties are being set.

3. Add "runc update" to "runc run [device cgroup deny]" test.

Fixes: 752e7a8
Fixes: 108ee85
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

libcontainer/cgroups/systemd/common.go

@@ -178,7 +178,7 @@ func allowAllDevices() []systemdDbus.Property {
 // corresponding set of systemd properties to configure the devices correctly.
 func generateDeviceProperties(r *configs.Resources) ([]systemdDbus.Property, error) {
 	if r.SkipDevices {
-		return allowAllDevices(), nil
+		return nil, nil
 	}
 
 	properties := []systemdDbus.Property{

libcontainer/cgroups/systemd/v1.go

@@ -339,27 +339,24 @@ func (m *legacyManager) Set(r *configs.Resources) error {
 		return err
 	}
 
+	// Figure out the current freezer state, so we can revert to it after we
+	// temporarily freeze the container.
+	targetFreezerState, err := m.GetFreezerState()
+	if err != nil {
+		return err
+	}
+	if targetFreezerState == configs.Undefined {
+		targetFreezerState = configs.Thawed
+	}
+
 	// We have to freeze the container while systemd sets the cgroup settings.
 	// The reason for this is that systemd's application of DeviceAllow rules
 	// is done disruptively, resulting in spurrious errors to common devices
 	// (unlike our fs driver, they will happily write deny-all rules to running
 	// containers). So we freeze the container to avoid them hitting the cgroup
 	// error. But if the freezer cgroup isn't supported, we just warn about it.
-	targetFreezerState := configs.Undefined
-	if !m.cgroups.SkipDevices {
-		// Figure out the current freezer state, so we can revert to it after we
-		// temporarily freeze the container.
-		targetFreezerState, err = m.GetFreezerState()
-		if err != nil {
-			return err
-		}
-		if targetFreezerState == configs.Undefined {
-			targetFreezerState = configs.Thawed
-		}
-
-		if err := m.Freeze(configs.Frozen); err != nil {
-			logrus.Infof("freeze container before SetUnitProperties failed: %v", err)
-		}
+	if err := m.Freeze(configs.Frozen); err != nil {
+		logrus.Infof("freeze container before SetUnitProperties failed: %v", err)
 	}
 
 	if err := setUnitProperties(m.dbus, getUnitName(m.cgroups), properties...); err != nil {

libcontainer/cgroups/systemd/v2.go

@@ -418,27 +418,24 @@ func (m *unifiedManager) Set(r *configs.Resources) error {
 		return err
 	}
 
+	// Figure out the current freezer state, so we can revert to it after we
+	// temporarily freeze the container.
+	targetFreezerState, err := m.GetFreezerState()
+	if err != nil {
+		return err
+	}
+	if targetFreezerState == configs.Undefined {
+		targetFreezerState = configs.Thawed
+	}
+
 	// We have to freeze the container while systemd sets the cgroup settings.
 	// The reason for this is that systemd's application of DeviceAllow rules
 	// is done disruptively, resulting in spurrious errors to common devices
 	// (unlike our fs driver, they will happily write deny-all rules to running
 	// containers). So we freeze the container to avoid them hitting the cgroup
 	// error. But if the freezer cgroup isn't supported, we just warn about it.
-	targetFreezerState := configs.Undefined
-	if !m.cgroups.SkipDevices {
-		// Figure out the current freezer state, so we can revert to it after we
-		// temporarily freeze the container.
-		targetFreezerState, err = m.GetFreezerState()
-		if err != nil {
-			return err
-		}
-		if targetFreezerState == configs.Undefined {
-			targetFreezerState = configs.Thawed
-		}
-
-		if err := m.Freeze(configs.Frozen); err != nil {
-			logrus.Infof("freeze container before SetUnitProperties failed: %v", err)
-		}
+	if err := m.Freeze(configs.Frozen); err != nil {
+		logrus.Infof("freeze container before SetUnitProperties failed: %v", err)
 	}
 
 	if err := setUnitProperties(m.dbus, getUnitName(m.cgroups), properties...); err != nil {

tests/integration/dev.bats

@@ -33,7 +33,7 @@ function teardown() {
 	[[ "${lines[0]}" =~ "crw-rw-rw".+"1".+"0".+"0".+"5,".+"2".+"/dev/ptmx" ]]
 }
 
-@test "runc run [device cgroup deny]" {
+@test "runc run/update [device cgroup deny]" {
 	requires root
 
 	update_config ' .linux.resources.devices = [{"allow": false, "access": "rwm"}]
@@ -52,6 +52,18 @@ function teardown() {
 	runc exec test_deny sh -c 'head -n 1 /dev/kmsg'
 	[ "$status" -eq 1 ]
 	[[ "${output}" == *'Operation not permitted'* ]]
+
+	runc update test_deny --pids-limit 42
+
+	# test write
+	runc exec test_deny sh -c 'hostname | tee /dev/kmsg'
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *'Operation not permitted'* ]]
+
+	# test read
+	runc exec test_deny sh -c 'head -n 1 /dev/kmsg'
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *'Operation not permitted'* ]]
 }
 
 @test "runc run [device cgroup allow rw char device]" {