diff --git a/libcontainer/cgroups/systemd/common.go b/libcontainer/cgroups/systemd/common.go index 67c612780f7..c9995bfe6c4 100644 --- a/libcontainer/cgroups/systemd/common.go +++ b/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{ diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 06526c54cf3..1f533c81747 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/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 { diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index cbd3a308e86..262aaca8524 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/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 { diff --git a/tests/integration/dev.bats b/tests/integration/dev.bats index 80ceb0a34bc..35a9ac0655f 100644 --- a/tests/integration/dev.bats +++ b/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]" {