diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index 164bf326f4b..4fc3951de91 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -3,13 +3,19 @@ package fs import ( + "bytes" + "fmt" + "reflect" + "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/cgroups/devices" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/system" ) type DevicesGroup struct { + testingSkipFinalCheck bool } func (s *DevicesGroup) Name() string { @@ -26,29 +32,74 @@ func (s *DevicesGroup) Apply(d *cgroupData) error { return nil } +func loadEmulator(path string) (*devices.Emulator, error) { + list, err := fscommon.ReadFile(path, "devices.list") + if err != nil { + return nil, err + } + return devices.EmulatorFromList(bytes.NewBufferString(list)) +} + +func buildEmulator(rules []*configs.DeviceRule) (*devices.Emulator, error) { + // This defaults to a white-list -- which is what we want! + emu := &devices.Emulator{} + for _, rule := range rules { + if err := emu.Apply(*rule); err != nil { + return nil, err + } + } + return emu, nil +} + func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error { if system.RunningInUserNS() { return nil } - // The devices list is a whitelist, so we must first deny all devices. - // XXX: This is incorrect for device list updates as it will result in - // spurrious errors in the container, but we will solve that - // separately. - if err := fscommon.WriteFile(path, "devices.deny", "a"); err != nil { + // Generate two emulators, one for the current state of the cgroup and one + // for the requested state by the user. + current, err := loadEmulator(path) + if err != nil { + return err + } + target, err := buildEmulator(cgroup.Resources.Devices) + if err != nil { return err } - devices := cgroup.Resources.Devices - for _, dev := range devices { + // Compute the minimal set of transition rules needed to achieve the + // requested state. + transitionRules, err := current.Transition(target) + if err != nil { + return err + } + for _, rule := range transitionRules { file := "devices.deny" - if dev.Allow { + if rule.Allow { file = "devices.allow" } - if err := fscommon.WriteFile(path, file, dev.CgroupString()); err != nil { + if err := fscommon.WriteFile(path, file, rule.CgroupString()); err != nil { return err } } + + // Final safety check -- ensure that the resulting state is what was + // requested. This is only really correct for white-lists, but for + // black-lists we can at least check that the cgroup is in the right mode. + // + // This safety-check is skipped for the unit tests because we cannot + // currently mock devices.list correctly. + if !s.testingSkipFinalCheck { + currentAfter, err := loadEmulator(path) + if err != nil { + return err + } + if !target.IsBlacklist() && !reflect.DeepEqual(currentAfter, target) { + return fmt.Errorf("resulting devices cgroup doesn't precisely match target") + } else if target.IsBlacklist() != currentAfter.IsBlacklist() { + return fmt.Errorf("resulting devices cgroup doesn't match target mode") + } + } return nil } diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index 77e29124cd4..da7ccb01ecc 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -16,6 +16,7 @@ func TestDevicesSetAllow(t *testing.T) { helper.writeFileContents(map[string]string{ "devices.allow": "", "devices.deny": "", + "devices.list": "a *:* rwm", }) helper.CgroupData.config.Resources.Devices = []*configs.DeviceRule{ @@ -28,7 +29,7 @@ func TestDevicesSetAllow(t *testing.T) { }, } - devices := &DevicesGroup{} + devices := &DevicesGroup{testingSkipFinalCheck: true} if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { t.Fatal(err) }