From dceeb0d0dfd45fc6258c335a777b58f0a935a87f Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Thu, 21 Jan 2016 15:27:20 -0800 Subject: [PATCH 1/2] Move pathClean to libcontainer/utils.CleanPath Signed-off-by: Kenfe-Mickael Laventure --- libcontainer/cgroups/fs/apply_raw.go | 27 ++------------------------- libcontainer/cgroups/fs/cpuset.go | 3 ++- libcontainer/rootfs_linux.go | 3 ++- libcontainer/utils/utils.go | 25 +++++++++++++++++++++++++ 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index 4da3b734..fd7077ed 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -14,6 +14,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" + libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" ) var ( @@ -267,30 +268,6 @@ func getCgroupPath(c *configs.Cgroup) (string, error) { return d.path("devices") } -// pathClean makes a path safe for use with filepath.Join. This is done by not -// only cleaning the path, but also (if the path is relative) adding a leading -// '/' and cleaning it (then removing the leading '/'). This ensures that a -// path resulting from prepending another path will always resolve to lexically -// be a subdirectory of the prefixed path. This is all done lexically, so paths -// that include symlinks won't be safe as a result of using pathClean. -func pathClean(path string) string { - // Ensure that all paths are cleaned (especially problematic ones like - // "/../../../../../" which can cause lots of issues). - path = filepath.Clean(path) - - // If the path isn't absolute, we need to do more processing to fix paths - // such as "../../../..//some/path". We also shouldn't convert absolute - // paths to relative ones. - if !filepath.IsAbs(path) { - path = filepath.Clean(string(os.PathSeparator) + path) - // This can't fail, as (by definition) all paths are relative to root. - path, _ = filepath.Rel(string(os.PathSeparator), path) - } - - // Clean the path again for good measure. - return filepath.Clean(path) -} - func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { root, err := getCgroupRoot() if err != nil { @@ -298,7 +275,7 @@ func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { } // Clean the parent slice path. - c.Parent = pathClean(c.Parent) + c.Parent = libcontainerUtils.CleanPath(c.Parent) return &cgroupData{ root: root, diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index ed100231..cbe62bd9 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -12,6 +12,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" + libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" ) type CpusetGroup struct { @@ -88,7 +89,7 @@ func (s *CpusetGroup) getSubsystemSettings(parent string) (cpus []byte, mems []b // it's parent. func (s *CpusetGroup) ensureParent(current, root string) error { parent := filepath.Dir(current) - if filepath.Clean(parent) == root { + if libcontainerUtils.CleanPath(parent) == root { return nil } // Avoid infinite recursion. diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index aa061ab0..5ea6ff01 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -19,6 +19,7 @@ import ( "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/label" "github.com/opencontainers/runc/libcontainer/system" + libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" ) const defaultMountFlags = syscall.MS_NOEXEC | syscall.MS_NOSUID | syscall.MS_NODEV @@ -294,7 +295,7 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) { // checkMountDestination checks to ensure that the mount destination is not over the top of /proc. // dest is required to be an abs path and have any symlinks resolved before calling this function. func checkMountDestination(rootfs, dest string) error { - if filepath.Clean(rootfs) == filepath.Clean(dest) { + if libcontainerUtils.CleanPath(rootfs) == libcontainerUtils.CleanPath(dest) { return fmt.Errorf("mounting into / is prohibited") } invalidDestinations := []string{ diff --git a/libcontainer/utils/utils.go b/libcontainer/utils/utils.go index 1378006b..1f5528ff 100644 --- a/libcontainer/utils/utils.go +++ b/libcontainer/utils/utils.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "encoding/json" "io" + "os" "path/filepath" "syscall" ) @@ -54,3 +55,27 @@ func WriteJSON(w io.Writer, v interface{}) error { _, err = w.Write(data) return err } + +// CleanPath makes a path safe for use with filepath.Join. This is done by not +// only cleaning the path, but also (if the path is relative) adding a leading +// '/' and cleaning it (then removing the leading '/'). This ensures that a +// path resulting from prepending another path will always resolve to lexically +// be a subdirectory of the prefixed path. This is all done lexically, so paths +// that include symlinks won't be safe as a result of using CleanPath. +func CleanPath(path string) string { + // Ensure that all paths are cleaned (especially problematic ones like + // "/../../../../../" which can cause lots of issues). + path = filepath.Clean(path) + + // If the path isn't absolute, we need to do more processing to fix paths + // such as "../../../..//some/path". We also shouldn't convert absolute + // paths to relative ones. + if !filepath.IsAbs(path) { + path = filepath.Clean(string(os.PathSeparator) + path) + // This can't fail, as (by definition) all paths are relative to root. + path, _ = filepath.Rel(string(os.PathSeparator), path) + } + + // Clean the path again for good measure. + return filepath.Clean(path) +} From 256f3a8ebc114b01c1630571c33d813a0612ea5c Mon Sep 17 00:00:00 2001 From: Kenfe-Mickael Laventure Date: Wed, 20 Jan 2016 18:04:59 -0800 Subject: [PATCH 2/2] Add support for CgroupsPath field Fixes #396 Signed-off-by: Kenfe-Mickael Laventure --- libcontainer/cgroups/fs/apply_raw.go | 36 ++++++++++++----------- libcontainer/configs/cgroup_unix.go | 15 +++++++--- libcontainer/integration/template_test.go | 3 +- spec.go | 20 +++++++++---- 4 files changed, 46 insertions(+), 28 deletions(-) diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index fd7077ed..21646e57 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -14,7 +14,6 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" - libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" ) var ( @@ -95,11 +94,10 @@ func getCgroupRoot() (string, error) { } type cgroupData struct { - root string - parent string - name string - config *configs.Cgroup - pid int + root string + innerPath string + config *configs.Cgroup + pid int } func (m *Manager) Apply(pid int) (err error) { @@ -274,15 +272,20 @@ func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { return nil, err } - // Clean the parent slice path. - c.Parent = libcontainerUtils.CleanPath(c.Parent) + if (c.Name != "" || c.Parent != "") && c.Path != "" { + return nil, fmt.Errorf("cgroup: either Path or Name and Parent should be used") + } + + innerPath := c.Path + if innerPath == "" { + innerPath = filepath.Join(c.Parent, c.Name) + } return &cgroupData{ - root: root, - parent: c.Parent, - name: c.Name, - config: c, - pid: pid, + root: root, + innerPath: c.Path, + config: c, + pid: pid, }, nil } @@ -310,11 +313,10 @@ func (raw *cgroupData) path(subsystem string) (string, error) { return "", err } - cgPath := filepath.Join(raw.parent, raw.name) // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. - if filepath.IsAbs(cgPath) { + if filepath.IsAbs(raw.innerPath) { // Sometimes subsystems can be mounted togethger as 'cpu,cpuacct'. - return filepath.Join(raw.root, filepath.Base(mnt), cgPath), nil + return filepath.Join(raw.root, filepath.Base(mnt), raw.innerPath), nil } parentPath, err := raw.parentPath(subsystem, mnt, root) @@ -322,7 +324,7 @@ func (raw *cgroupData) path(subsystem string) (string, error) { return "", err } - return filepath.Join(parentPath, cgPath), nil + return filepath.Join(parentPath, raw.innerPath), nil } func (raw *cgroupData) join(subsystem string) (string, error) { diff --git a/libcontainer/configs/cgroup_unix.go b/libcontainer/configs/cgroup_unix.go index b103e269..40a033f3 100644 --- a/libcontainer/configs/cgroup_unix.go +++ b/libcontainer/configs/cgroup_unix.go @@ -11,15 +11,22 @@ const ( ) type Cgroup struct { - Name string `json:"name"` + // Deprecated, use Path instead + Name string `json:"name,omitempty"` - // name of parent cgroup or slice - Parent string `json:"parent"` + // name of parent of cgroup or slice + // Deprecated, use Path instead + Parent string `json:"parent,omitempty"` + + // Path specifies the path to cgroups that are created and/or joined by the container. + // The path is assumed to be relative to the host system cgroup mountpoint. + Path string `json:"path"` // ScopePrefix decribes prefix for the scope name ScopePrefix string `json:"scope_prefix"` - // Paths represent the cgroups paths to join + // Paths represent the absolute cgroups paths to join. + // This takes precedence over Path. Paths map[string]string // Resources contains various cgroups settings to apply diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go index 75f5861a..047a5f1e 100644 --- a/libcontainer/integration/template_test.go +++ b/libcontainer/integration/template_test.go @@ -46,8 +46,7 @@ func newTemplateConfig(rootfs string) *configs.Config { {Type: configs.NEWNET}, }), Cgroups: &configs.Cgroup{ - Name: "test", - Parent: "integration", + Path: "integration/test", Resources: &configs.Resources{ MemorySwappiness: -1, AllowAllDevices: false, diff --git a/spec.go b/spec.go index 74e18fa8..2836e7c8 100644 --- a/spec.go +++ b/spec.go @@ -18,6 +18,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/seccomp" + libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" "github.com/opencontainers/specs" ) @@ -326,13 +327,22 @@ func createLibcontainerMount(cwd string, m specs.Mount) *configs.Mount { } func createCgroupConfig(name string, spec *specs.LinuxSpec) (*configs.Cgroup, error) { - myCgroupPath, err := cgroups.GetThisCgroupDir("devices") - if err != nil { - return nil, err + var ( + err error + myCgroupPath string + ) + + if spec.Linux.CgroupsPath != nil { + myCgroupPath = libcontainerUtils.CleanPath(*spec.Linux.CgroupsPath) + } else { + myCgroupPath, err = cgroups.GetThisCgroupDir("devices") + if err != nil { + return nil, err + } } + c := &configs.Cgroup{ - Name: name, - Parent: myCgroupPath, + Path: filepath.Join(myCgroupPath, name), Resources: &configs.Resources{}, } c.Resources.AllowedDevices = allowedDevices