From ef4ff6a8ad9b70b02e92c3713fe7ae69655a5be0 Mon Sep 17 00:00:00 2001 From: Buddha Prakash Date: Wed, 20 Jul 2016 10:46:11 -0700 Subject: [PATCH 1/2] Skip updates on parent Devices cgroup Signed-off-by: Buddha Prakash --- libcontainer/README.md | 2 +- libcontainer/cgroups/fs/devices.go | 26 ++++++++++++----------- libcontainer/cgroups/fs/devices_test.go | 7 +++--- libcontainer/configs/cgroup_unix.go | 2 +- libcontainer/integration/template_test.go | 3 ++- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/libcontainer/README.md b/libcontainer/README.md index 6331010f..457b132e 100644 --- a/libcontainer/README.md +++ b/libcontainer/README.md @@ -77,7 +77,7 @@ config := &configs.Config{ Parent: "system", Resources: &configs.Resources{ MemorySwappiness: nil, - AllowAllDevices: false, + AllowAllDevices: nil, AllowedDevices: configs.DefaultAllowedDevices, }, }, diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index 5f783310..0ac5b4ed 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -43,21 +43,23 @@ func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error { } return nil } - if !cgroup.Resources.AllowAllDevices { - if err := writeFile(path, "devices.deny", "a"); err != nil { - return err - } - - for _, dev := range cgroup.Resources.AllowedDevices { - if err := writeFile(path, "devices.allow", dev.CgroupString()); err != nil { + if cgroup.Resources.AllowAllDevices != nil { + if *cgroup.Resources.AllowAllDevices == false { + if err := writeFile(path, "devices.deny", "a"); err != nil { return err } - } - return nil - } - if err := writeFile(path, "devices.allow", "a"); err != nil { - return err + for _, dev := range cgroup.Resources.AllowedDevices { + if err := writeFile(path, "devices.allow", dev.CgroupString()); err != nil { + return err + } + } + return nil + } + + if err := writeFile(path, "devices.allow", "a"); err != nil { + return err + } } for _, dev := range cgroup.Resources.DeniedDevices { diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index ee44084e..c27d3c3e 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -40,8 +40,8 @@ func TestDevicesSetAllow(t *testing.T) { helper.writeFileContents(map[string]string{ "devices.deny": "a", }) - - helper.CgroupData.config.Resources.AllowAllDevices = false + allowAllDevices := false + helper.CgroupData.config.Resources.AllowAllDevices = &allowAllDevices helper.CgroupData.config.Resources.AllowedDevices = allowedDevices devices := &DevicesGroup{} if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { @@ -66,7 +66,8 @@ func TestDevicesSetDeny(t *testing.T) { "devices.allow": "a", }) - helper.CgroupData.config.Resources.AllowAllDevices = true + allowAllDevices := true + helper.CgroupData.config.Resources.AllowAllDevices = &allowAllDevices helper.CgroupData.config.Resources.DeniedDevices = deniedDevices devices := &DevicesGroup{} if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { diff --git a/libcontainer/configs/cgroup_unix.go b/libcontainer/configs/cgroup_unix.go index abbb92f5..bd6f69b8 100644 --- a/libcontainer/configs/cgroup_unix.go +++ b/libcontainer/configs/cgroup_unix.go @@ -36,7 +36,7 @@ type Cgroup struct { type Resources struct { // If this is true allow access to any kind of device within the container. If false, allow access only to devices explicitly listed in the allowed_devices list. // Deprecated - AllowAllDevices bool `json:"allow_all_devices,omitempty"` + AllowAllDevices *bool `json:"allow_all_devices,omitempty"` // Deprecated AllowedDevices []*Device `json:"allowed_devices,omitempty"` // Deprecated diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go index 96165684..774032f5 100644 --- a/libcontainer/integration/template_test.go +++ b/libcontainer/integration/template_test.go @@ -20,6 +20,7 @@ const defaultMountFlags = syscall.MS_NOEXEC | syscall.MS_NOSUID | syscall.MS_NOD // it uses a network strategy of just setting a loopback interface // and the default setup for devices func newTemplateConfig(rootfs string) *configs.Config { + allowAllDevices := false return &configs.Config{ Rootfs: rootfs, Capabilities: []string{ @@ -49,7 +50,7 @@ func newTemplateConfig(rootfs string) *configs.Config { Path: "integration/test", Resources: &configs.Resources{ MemorySwappiness: nil, - AllowAllDevices: false, + AllowAllDevices: &allowAllDevices, AllowedDevices: configs.DefaultAllowedDevices, }, }, From d4c67195c62e56f8e2fae336167f9c1bce30d7bc Mon Sep 17 00:00:00 2001 From: Buddha Prakash Date: Fri, 22 Jul 2016 18:51:10 -0700 Subject: [PATCH 2/2] Add test Signed-off-by: Buddha Prakash --- libcontainer/cgroups/fs/devices_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index c27d3c3e..fc635b99 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -56,6 +56,19 @@ func TestDevicesSetAllow(t *testing.T) { if value != allowedList { t.Fatal("Got the wrong value, set devices.allow failed.") } + + // When AllowAllDevices is nil, devices.allow file should not be modified. + helper.CgroupData.config.Resources.AllowAllDevices = nil + if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil { + t.Fatal(err) + } + value, err = getCgroupParamString(helper.CgroupPath, "devices.allow") + if err != nil { + t.Fatalf("Failed to parse devices.allow - %s", err) + } + if value != allowedList { + t.Fatal("devices policy shouldn't have changed on AllowedAllDevices=nil.") + } } func TestDevicesSetDeny(t *testing.T) {