From 5e1dcdf564530598f986cdd3310354917fca9b02 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 13 Feb 2025 11:56:43 -0800 Subject: [PATCH] libct/cg: add internal/path.Inner The code which determines inner cgroup path from cgroup config is identical in fs and fs2 drivers, and it is using utils.CleanPath. In preparation to move libcontainer/cgroups to a separate repo, we have to get rid of libcontainer/utils dependency. So, - copy the utils.CleanPath implementation to internal/path; - consolidate the two innerPath implementations to internal/path. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/paths.go | 20 +-------- libcontainer/cgroups/fs/paths_test.go | 3 +- libcontainer/cgroups/fs2/defaultpath.go | 15 ++----- libcontainer/cgroups/internal/path/path.go | 52 ++++++++++++++++++++++ 4 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 libcontainer/cgroups/internal/path/path.go diff --git a/libcontainer/cgroups/fs/paths.go b/libcontainer/cgroups/fs/paths.go index b8516865a..59cd13d5c 100644 --- a/libcontainer/cgroups/fs/paths.go +++ b/libcontainer/cgroups/fs/paths.go @@ -9,7 +9,7 @@ import ( "golang.org/x/sys/unix" "github.com/opencontainers/runc/libcontainer/cgroups" - "github.com/opencontainers/runc/libcontainer/utils" + "github.com/opencontainers/runc/libcontainer/cgroups/internal/path" ) // The absolute path to the root of the cgroup hierarchies. @@ -26,7 +26,7 @@ func initPaths(cg *cgroups.Cgroup) (map[string]string, error) { return nil, err } - inner, err := innerPath(cg) + inner, err := path.Inner(cg) if err != nil { return nil, err } @@ -135,22 +135,6 @@ func rootPath() (string, error) { return cgroupRoot, nil } -func innerPath(c *cgroups.Cgroup) (string, error) { - if (c.Name != "" || c.Parent != "") && c.Path != "" { - return "", errors.New("cgroup: either Path or Name and Parent should be used") - } - - // XXX: Do not remove CleanPath. Path safety is important! -- cyphar - innerPath := utils.CleanPath(c.Path) - if innerPath == "" { - cgParent := utils.CleanPath(c.Parent) - cgName := utils.CleanPath(c.Name) - innerPath = filepath.Join(cgParent, cgName) - } - - return innerPath, nil -} - func subsysPath(root, inner, subsystem string) (string, error) { // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. if filepath.IsAbs(inner) { diff --git a/libcontainer/cgroups/fs/paths_test.go b/libcontainer/cgroups/fs/paths_test.go index 43dcfcbbb..692ff5e56 100644 --- a/libcontainer/cgroups/fs/paths_test.go +++ b/libcontainer/cgroups/fs/paths_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/cgroups/internal/path" ) func TestInvalidCgroupPath(t *testing.T) { @@ -66,7 +67,7 @@ func TestInvalidCgroupPath(t *testing.T) { t.Run(tc.test, func(t *testing.T) { config := &cgroups.Cgroup{Path: tc.path, Name: tc.name, Parent: tc.parent} - inner, err := innerPath(config) + inner, err := path.Inner(config) if err != nil { t.Fatalf("couldn't get cgroup data: %v", err) } diff --git a/libcontainer/cgroups/fs2/defaultpath.go b/libcontainer/cgroups/fs2/defaultpath.go index 451f2e08d..6a08cceaf 100644 --- a/libcontainer/cgroups/fs2/defaultpath.go +++ b/libcontainer/cgroups/fs2/defaultpath.go @@ -25,22 +25,15 @@ import ( "strings" "github.com/opencontainers/runc/libcontainer/cgroups" - "github.com/opencontainers/runc/libcontainer/utils" + "github.com/opencontainers/runc/libcontainer/cgroups/internal/path" ) const UnifiedMountpoint = "/sys/fs/cgroup" func defaultDirPath(c *cgroups.Cgroup) (string, error) { - if (c.Name != "" || c.Parent != "") && c.Path != "" { - return "", errors.New("cgroup: either Path or Name and Parent should be used") - } - - // XXX: Do not remove CleanPath. Path safety is important! -- cyphar - innerPath := utils.CleanPath(c.Path) - if innerPath == "" { - cgParent := utils.CleanPath(c.Parent) - cgName := utils.CleanPath(c.Name) - innerPath = filepath.Join(cgParent, cgName) + innerPath, err := path.Inner(c) + if err != nil { + return "", err } if filepath.IsAbs(innerPath) { diff --git a/libcontainer/cgroups/internal/path/path.go b/libcontainer/cgroups/internal/path/path.go new file mode 100644 index 000000000..94e491791 --- /dev/null +++ b/libcontainer/cgroups/internal/path/path.go @@ -0,0 +1,52 @@ +package path + +import ( + "errors" + "os" + "path/filepath" + + "github.com/opencontainers/runc/libcontainer/cgroups" +) + +// Inner returns a path to cgroup relative to a cgroup mount point, based +// on cgroup configuration, or an error, if cgroup configuration is invalid. +// To be used only by fs cgroup managers (systemd has different path rules). +func Inner(c *cgroups.Cgroup) (string, error) { + if (c.Name != "" || c.Parent != "") && c.Path != "" { + return "", errors.New("cgroup: either Path or Name and Parent should be used") + } + + // XXX: Do not remove cleanPath. Path safety is important! -- cyphar + innerPath := cleanPath(c.Path) + if innerPath == "" { + cgParent := cleanPath(c.Parent) + cgName := cleanPath(c.Name) + innerPath = filepath.Join(cgParent, cgName) + } + + return innerPath, nil +} + +// cleanPath is a copy of github.com/opencontainers/runc/libcontainer/utils.CleanPath. +func cleanPath(path string) string { + // Deal with empty strings nicely. + if path == "" { + return "" + } + + // Ensure that all paths are cleaned (especially problematic ones like + // "/../../../../../" which can cause lots of issues). + + if filepath.IsAbs(path) { + return 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. + 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) + + return path +}