merge CVE-2026-41579 fixes into release-1.3

Aleksa Sarai (8):
  rootfs: make cgroupv1 subsystem symlinks fd-based
  rootfs: make /dev initialisation code fd-based
  rootfs: switch createDevices argument order
  pathrs: add MkdirAllParentInRoot helper
  pathrs: add "hallucination" helpers for SecureJoin magic
  pathrs: rename MkdirAllInRootOpen -> MkdirAllInRoot
  libct: switch final WithProcfd users to WithProcfdFile
  libcontainer: move CleanPath and StripRoot to internal/pathrs

Li Fubang (2):
  integration: add some tests for bind mount through dangling symlinks
  libct: use preopened rootfs more

Kir Kolyshkin (3):
  Pre-open container root directory
  libct: minor refactor in mountToRootfs
  libct: mountCgroupV1: address TODO

LGTMs: lifubang kolyshkin rata

internal/pathrs/mkdirall.go

@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: Apache-2.0
+/*
+ * Copyright (C) 2024-2025 Aleksa Sarai <cyphar@cyphar.com>
+ * Copyright (C) 2024-2025 SUSE LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package pathrs
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+func splitPath(path string) (dirPath, filename string, err error) {
+	dirPath, filename = filepath.Split(path)
+	if filepath.Join("/", filename) == "/" {
+		return "", "", fmt.Errorf("root subpath %q has bad trailing component %q", path, filename)
+	}
+	return dirPath, filename, nil
+}
+
+// MkdirAllParentInRoot is like [MkdirAllInRoot] except that it only creates
+// the parent directory of the target path, returning the trailing component so
+// the caller has more flexibility around constructing the final inode.
+//
+// Callers need to be very careful operating on the trailing path, as trivial
+// mistakes like following symlinks can cause security bugs. Most people
+// should probably just use [MkdirAllInRoot] or [CreateInRoot].
+func MkdirAllParentInRoot(root *os.File, unsafePath string, mode os.FileMode) (*os.File, string, error) {
+	// MkdirAllInRoot also does hallucinateUnsafePath, but we need to do it
+	// here first because when we split unsafePath into (dir, file) components
+	// we want to be doing so with the hallucinated path (so that trailing
+	// dangling symlinks are treated correctly).
+	unsafePath, err := hallucinateUnsafePath(root.Name(), unsafePath)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to construct hallucinated target path: %w", err)
+	}
+
+	dirPath, filename, err := splitPath(unsafePath)
+	if err != nil {
+		return nil, "", fmt.Errorf("split path %q for mkdir parent: %w", unsafePath, err)
+	}
+
+	dirFd, err := MkdirAllInRoot(root, dirPath, mode)
+	return dirFd, filename, err
+}

internal/pathrs/mkdirall_pathrslite.go

@@ -21,16 +21,14 @@ package pathrs
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"github.com/cyphar/filepath-securejoin/pathrs-lite"
 	"github.com/sirupsen/logrus"
-	"golang.org/x/sys/unix"
 )
 
-// MkdirAllInRootOpen attempts to make
+// MkdirAllInRoot attempts to make
 //
-//	path, _ := securejoin.SecureJoin(root, unsafePath)
+//	path, _ := securejoin.SecureJoin(root.Name(), unsafePath)
 //	os.MkdirAll(path, mode)
 //	os.Open(path)
 //
@@ -49,19 +47,10 @@ import (
 // handling if unsafePath has already been scoped within the rootfs (this is
 // needed for a lot of runc callers and fixing this would require reworking a
 // lot of path logic).
-func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (*os.File, error) {
-	// If the path is already "within" the root, get the path relative to the
-	// root and use that as the unsafe path. This is necessary because a lot of
-	// MkdirAllInRootOpen callers have already done SecureJoin, and refactoring
-	// all of them to stop using these SecureJoin'd paths would require a fair
-	// amount of work.
-	// TODO(cyphar): Do the refactor to libpathrs once it's ready.
-	if IsLexicallyInRoot(root, unsafePath) {
-		subPath, err := filepath.Rel(root, unsafePath)
-		if err != nil {
-			return nil, err
-		}
-		unsafePath = subPath
+func MkdirAllInRoot(root *os.File, unsafePath string, mode os.FileMode) (*os.File, error) {
+	unsafePath, err := hallucinateUnsafePath(root.Name(), unsafePath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to construct hallucinated target path: %w", err)
 	}
 
 	// Check for any silly mode bits.
@@ -77,23 +66,7 @@ func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (*os.File, er
 		mode &= 0o1777
 	}
 
-	rootDir, err := os.OpenFile(root, unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
-	if err != nil {
-		return nil, fmt.Errorf("open root handle: %w", err)
-	}
-	defer rootDir.Close()
-
 	return retryEAGAIN(func() (*os.File, error) {
-		return pathrs.MkdirAllHandle(rootDir, unsafePath, mode)
+		return pathrs.MkdirAllHandle(root, unsafePath, mode)
 	})
 }
-
-// MkdirAllInRoot is a wrapper around MkdirAllInRootOpen which closes the
-// returned handle, for callers that don't need to use it.
-func MkdirAllInRoot(root, unsafePath string, mode os.FileMode) error {
-	f, err := MkdirAllInRootOpen(root, unsafePath, mode)
-	if err == nil {
-		_ = f.Close()
-	}
-	return err
-}

internal/pathrs/path.go

@@ -19,7 +19,11 @@
 package pathrs
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
+
+	securejoin "github.com/cyphar/filepath-securejoin"
 )
 
 // IsLexicallyInRoot is shorthand for strings.HasPrefix(path+"/", root+"/"),
@@ -32,3 +36,81 @@ func IsLexicallyInRoot(root, path string) bool {
 	path = strings.TrimRight(path, "/")
 	return strings.HasPrefix(path+"/", root+"/")
 }
+
+// LexicallyCleanPath 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 LexicallyCleanPath(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 "../../../../<etc>/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
+}
+
+// LexicallyStripRoot returns the passed path, stripping the root path if it
+// was (lexicially) inside it. Note that both passed paths will always be
+// treated as absolute, and the returned path will also always be absolute. In
+// addition, the paths are cleaned before stripping the root.
+func LexicallyStripRoot(root, path string) string {
+	// Make the paths clean and absolute.
+	root, path = LexicallyCleanPath("/"+root), LexicallyCleanPath("/"+path)
+	switch {
+	case path == root:
+		path = "/"
+	case root == "/":
+		// do nothing
+	default:
+		path = strings.TrimPrefix(path, root+"/")
+	}
+	return LexicallyCleanPath("/" + path)
+}
+
+// hallucinateUnsafePath creates a new unsafePath which has all symlinks
+// (including dangling symlinks) fully resolved and any non-existent components
+// treated as though they are real. This is effectively just a wrapper around
+// [securejoin.SecureJoin] that strips the root. This path *IS NOT* safe to use
+// as-is, you *MUST* operate on the returned path with pathrs-lite.
+//
+// The reason for this methods is that in previous runc versions, we would
+// tolerate nonsense paths with dangling symlinks as path components.
+// pathrs-lite does not support this, so instead we have to emulate this
+// behaviour by doing SecureJoin *purely to get a semi-reasonable path to use*
+// and then we use pathrs-lite to operate on the path safely.
+//
+// It would be quite difficult to emulate this in a race-free way in
+// pathrs-lite, so instead we use [securejoin.SecureJoin] to simply produce a
+// new candidate path for operations like [MkdirAllInRoot] so they can then
+// operate on the new unsafePath as if it was what the user requested.
+//
+// If unsafePath is already lexically inside root, it is stripped before
+// re-resolving it (this is done to ensure compatibility with legacy callers
+// within runc that call SecureJoin before calling into pathrs).
+func hallucinateUnsafePath(root, unsafePath string) (string, error) {
+	unsafePath = LexicallyStripRoot(root, unsafePath)
+	weirdPath, err := securejoin.SecureJoin(root, unsafePath)
+	if err != nil {
+		return "", err
+	}
+	unsafePath = LexicallyStripRoot(root, weirdPath)
+	return unsafePath, nil
+}

internal/pathrs/path_test.go

@@ -51,3 +51,70 @@ func TestIsLexicallyInRoot(t *testing.T) {
 		})
 	}
 }
+
+func TestLexicallyCleanPath(t *testing.T) {
+	path := LexicallyCleanPath("")
+	if path != "" {
+		t.Errorf("expected to receive empty string and received %s", path)
+	}
+
+	path = LexicallyCleanPath("rootfs")
+	if path != "rootfs" {
+		t.Errorf("expected to receive 'rootfs' and received %s", path)
+	}
+
+	path = LexicallyCleanPath("../../../var")
+	if path != "var" {
+		t.Errorf("expected to receive 'var' and received %s", path)
+	}
+
+	path = LexicallyCleanPath("/../../../var")
+	if path != "/var" {
+		t.Errorf("expected to receive '/var' and received %s", path)
+	}
+
+	path = LexicallyCleanPath("/foo/bar/")
+	if path != "/foo/bar" {
+		t.Errorf("expected to receive '/foo/bar' and received %s", path)
+	}
+
+	path = LexicallyCleanPath("/foo/bar/../")
+	if path != "/foo" {
+		t.Errorf("expected to receive '/foo' and received %s", path)
+	}
+}
+
+func TestLexicallyStripRoot(t *testing.T) {
+	for _, test := range []struct {
+		root, path, out string
+	}{
+		// Works with multiple components.
+		{"/a/b", "/a/b/c", "/c"},
+		{"/hello/world", "/hello/world/the/quick-brown/fox", "/the/quick-brown/fox"},
+		// '/' must be a no-op.
+		{"/", "/a/b/c", "/a/b/c"},
+		// Must be the correct order.
+		{"/a/b", "/a/c/b", "/a/c/b"},
+		// Must be at start.
+		{"/abc/def", "/foo/abc/def/bar", "/foo/abc/def/bar"},
+		// Must be a lexical parent.
+		{"/foo/bar", "/foo/barSAMECOMPONENT", "/foo/barSAMECOMPONENT"},
+		// Must only strip the root once.
+		{"/foo/bar", "/foo/bar/foo/bar/baz", "/foo/bar/baz"},
+		// Deal with .. in a fairly sane way.
+		{"/foo/bar", "/foo/bar/../baz", "/foo/baz"},
+		{"/foo/bar", "../../../../../../foo/bar/baz", "/baz"},
+		{"/foo/bar", "/../../../../../../foo/bar/baz", "/baz"},
+		{"/foo/bar/../baz", "/foo/baz/bar", "/bar"},
+		{"/foo/bar/../baz", "/foo/baz/../bar/../baz/./foo", "/foo"},
+		// All paths are made absolute before stripping.
+		{"foo/bar", "/foo/bar/baz/bee", "/baz/bee"},
+		{"/foo/bar", "foo/bar/baz/beef", "/baz/beef"},
+		{"foo/bar", "foo/bar/baz/beets", "/baz/beets"},
+	} {
+		got := LexicallyStripRoot(test.root, test.path)
+		if got != test.out {
+			t.Errorf("LexicallyStripRoot(%q, %q) -- got %q, expected %q", test.root, test.path, got, test.out)
+		}
+	}
+}

internal/pathrs/root_pathrslite.go

@@ -28,11 +28,11 @@ import (
 )
 
 // OpenInRoot opens the given path inside the root with the provided flags. It
-// is effectively shorthand for [securejoin.OpenInRoot] followed by
+// is effectively shorthand for [securejoin.OpenatInRoot] followed by
 // [securejoin.Reopen].
-func OpenInRoot(root, subpath string, flags int) (*os.File, error) {
+func OpenInRoot(root *os.File, subpath string, flags int) (*os.File, error) {
 	handle, err := retryEAGAIN(func() (*os.File, error) {
-		return pathrs.OpenInRoot(root, subpath)
+		return pathrs.OpenatInRoot(root, subpath)
 	})
 	if err != nil {
 		return nil, err
@@ -47,13 +47,8 @@ func OpenInRoot(root, subpath string, flags int) (*os.File, error) {
 // open(O_CREAT|O_NOFOLLOW) semantics. If you want the creation to use O_EXCL,
 // include it in the passed flags. The fileMode argument uses unix.* mode bits,
 // *not* os.FileMode.
-func CreateInRoot(root, subpath string, flags int, fileMode uint32) (*os.File, error) {
-	dir, filename := filepath.Split(subpath)
-	if filepath.Join("/", filename) == "/" {
-		return nil, fmt.Errorf("create in root subpath %q has bad trailing component %q", subpath, filename)
-	}
-
-	dirFd, err := MkdirAllInRootOpen(root, dir, 0o755)
+func CreateInRoot(root *os.File, subpath string, flags int, fileMode uint32) (*os.File, error) {
+	dirFd, filename, err := MkdirAllParentInRoot(root, subpath, 0o755)
 	if err != nil {
 		return nil, err
 	}
@@ -68,5 +63,48 @@ func CreateInRoot(root, subpath string, flags int, fileMode uint32) (*os.File, e
 	if err != nil {
 		return nil, err
 	}
-	return os.NewFile(uintptr(fd), root+"/"+subpath), nil
+	return os.NewFile(uintptr(fd), root.Name()+"/"+subpath), nil
+}
+
+// UnlinkInRoot deletes the inode specified at the given subpath. If you pass
+// [unix.AT_REMOVEDIR] it will remove directories, otherwise it will remove
+// non-directory inodes.
+func UnlinkInRoot(root *os.File, subpath string, flags int) error {
+	dirPath, filename, err := splitPath(subpath)
+	if err != nil {
+		return fmt.Errorf("split path %q for unlink: %w", subpath, err)
+	}
+
+	dirFd := root
+	if filepath.Join("/", dirPath) != "/" {
+		newDirFd, err := OpenInRoot(root, dirPath, unix.O_DIRECTORY|unix.O_PATH)
+		if err != nil {
+			return fmt.Errorf("failed to open parent directory %q for unlink: %w", dirPath, err)
+		}
+		dirFd = newDirFd
+		defer dirFd.Close()
+	}
+
+	err = unix.Unlinkat(int(dirFd.Fd()), filename, flags)
+	if err != nil {
+		err = &os.PathError{Op: "unlinkat", Path: dirFd.Name() + "/" + filename, Err: err}
+	}
+	return err
+}
+
+// SymlinkInRoot creates a symlink inside a root with the given target (as well
+// as creating any missing parent directories). If the subpath already exists,
+// an error is returned.
+func SymlinkInRoot(linktarget string, root *os.File, subpath string) error {
+	dirFd, filename, err := MkdirAllParentInRoot(root, subpath, 0o755)
+	if err != nil {
+		return err
+	}
+	defer dirFd.Close()
+
+	err = unix.Symlinkat(linktarget, int(dirFd.Fd()), filename)
+	if err != nil {
+		err = &os.PathError{Op: "symlinkat", Path: dirFd.Name() + "/" + filename, Err: err}
+	}
+	return err
 }

libcontainer/apparmor/apparmor_linux.go

@@ -9,7 +9,6 @@ import (
 	"golang.org/x/sys/unix"
 
 	"github.com/opencontainers/runc/internal/pathrs"
-	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
 var (
@@ -29,7 +28,7 @@ func isEnabled() bool {
 }
 
 func setProcAttr(attr, value string) error {
-	attr = utils.CleanPath(attr)
+	attr = pathrs.LexicallyCleanPath(attr)
 	attrSubPath := "attr/apparmor/" + attr
 	if _, err := os.Stat("/proc/self/" + attrSubPath); errors.Is(err, os.ErrNotExist) {
 		// fall back to the old convention

libcontainer/criu_linux.go

@@ -24,6 +24,7 @@ import (
 	"google.golang.org/protobuf/proto"
 
 	"github.com/opencontainers/cgroups"
+	"github.com/opencontainers/runc/internal/pathrs"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/utils"
 )
@@ -539,19 +540,31 @@ func isOnTmpfs(path string, mounts []*configs.Mount) bool {
 // This function also creates missing mountpoints as long as they
 // are not on top of a tmpfs, as CRIU will restore tmpfs content anyway.
 func (c *Container) prepareCriuRestoreMounts(mounts []*configs.Mount) error {
+	rootFd, err := os.OpenFile(c.config.Rootfs, unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_PATH, 0)
+	if err != nil {
+		return fmt.Errorf("open rootfs handle: %w", err)
+	}
+	defer rootFd.Close()
+
 	umounts := []string{}
 	defer func() {
 		for _, u := range umounts {
-			_ = utils.WithProcfd(c.config.Rootfs, u, func(procfd string) error {
-				if e := unix.Unmount(procfd, unix.MNT_DETACH); e != nil {
-					if e != unix.EINVAL {
+			mntFile, err := pathrs.OpenInRoot(rootFd, u, unix.O_PATH)
+			if err != nil {
+				logrus.Warnf("Error during cleanup unmounting %s: open handle: %v", u, err)
+				continue
+			}
+			_ = utils.WithProcfdFile(mntFile, func(procfd string) error {
+				if err := unix.Unmount(procfd, unix.MNT_DETACH); err != nil {
+					if err != unix.EINVAL {
 						// Ignore EINVAL as it means 'target is not a mount point.'
 						// It probably has already been unmounted.
-						logrus.Warnf("Error during cleanup unmounting of %s (%s): %v", procfd, u, e)
+						logrus.Warnf("Error during cleanup unmounting of %s (%s): %v", procfd, u, err)
 					}
 				}
 				return nil
 			})
+			_ = mntFile.Close()
 		}
 	}()
 	// Now go through all mounts and create the required mountpoints.
@@ -570,7 +583,7 @@ func (c *Container) prepareCriuRestoreMounts(mounts []*configs.Mount) error {
 			continue
 		}
 		me := mountEntry{Mount: m}
-		if err := me.createOpenMountpoint(c.config.Rootfs); err != nil {
+		if err := me.createOpenMountpoint(rootFd); err != nil {
 			return fmt.Errorf("create criu restore mountpoint for %s mount: %w", me.Destination, err)
 		}
 		if me.dstFile != nil {

libcontainer/factory_linux.go

@@ -11,10 +11,10 @@ import (
 
 	"github.com/opencontainers/cgroups"
 	"github.com/opencontainers/cgroups/manager"
+	"github.com/opencontainers/runc/internal/pathrs"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/configs/validate"
 	"github.com/opencontainers/runc/libcontainer/intelrdt"
-	"github.com/opencontainers/runc/libcontainer/utils"
 )
 
 const (
@@ -211,7 +211,7 @@ func validateID(id string) error {
 
 	}
 
-	if string(os.PathSeparator)+id != utils.CleanPath(string(os.PathSeparator)+id) {
+	if string(os.PathSeparator)+id != pathrs.LexicallyCleanPath(string(os.PathSeparator)+id) {
 		return ErrInvalidID
 	}
 

libcontainer/rootfs_linux.go

@@ -12,7 +12,6 @@ import (
 	"syscall"
 	"time"
 
-	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/cyphar/filepath-securejoin/pathrs-lite/procfs"
 	"github.com/moby/sys/mountinfo"
 	"github.com/moby/sys/userns"
@@ -35,7 +34,7 @@ const defaultMountFlags = unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NODEV
 
 // mountConfig contains mount data not specific to a mount point.
 type mountConfig struct {
-	root            string
+	root            *os.File
 	label           string
 	cgroup2Path     string
 	rootlessCgroups bool
@@ -90,13 +89,26 @@ func (m mountEntry) srcStatfs() (*unix.Statfs_t, error) {
 // needsSetupDev returns true if /dev needs to be set up.
 func needsSetupDev(config *configs.Config) bool {
 	for _, m := range config.Mounts {
-		if m.Device == "bind" && utils.CleanPath(m.Destination) == "/dev" {
+		if m.Device == "bind" && pathrs.LexicallyCleanPath(m.Destination) == "/dev" {
 			return false
 		}
 	}
 	return true
 }
 
+func doSetupDev(rootFd *os.File, config *configs.Config) error {
+	if err := createDevices(rootFd, config); err != nil {
+		return fmt.Errorf("error creating device nodes: %w", err)
+	}
+	if err := setupPtmx(rootFd); err != nil {
+		return fmt.Errorf("error setting up ptmx: %w", err)
+	}
+	if err := setupDevSymlinks(rootFd); err != nil {
+		return fmt.Errorf("error setting up /dev symlinks: %w", err)
+	}
+	return nil
+}
+
 // prepareRootfs sets up the devices, mount points, and filesystems for use
 // inside a new mount namespace. It doesn't set anything as ro. You must call
 // finalizeRootfs after this function to finish setting up the rootfs.
@@ -106,8 +118,17 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 		return fmt.Errorf("error preparing rootfs: %w", err)
 	}
 
+	// Pre-open rootfs. NOTE that if we need to re-enable support for mounting
+	// on top of container root (see issue 5070), we will need to reopen rootFd
+	// after such mounts.
+	rootFd, err := os.OpenFile(config.Rootfs, unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_PATH, 0)
+	if err != nil {
+		return fmt.Errorf("open rootfs handle: %w", err)
+	}
+	defer rootFd.Close()
+
 	mountConfig := &mountConfig{
-		root:            config.Rootfs,
+		root:            rootFd,
 		label:           config.MountLabel,
 		cgroup2Path:     iConfig.Cgroup2Path,
 		rootlessCgroups: config.RootlessCgroups,
@@ -167,14 +188,8 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 
 	setupDev := needsSetupDev(config)
 	if setupDev {
-		if err := createDevices(config); err != nil {
-			return fmt.Errorf("error creating device nodes: %w", err)
-		}
-		if err := setupPtmx(config); err != nil {
-			return fmt.Errorf("error setting up ptmx: %w", err)
-		}
-		if err := setupDevSymlinks(config.Rootfs); err != nil {
-			return fmt.Errorf("error setting up /dev symlinks: %w", err)
+		if err := doSetupDev(rootFd, config); err != nil {
+			return fmt.Errorf("configuring container /dev: %w", err)
 		}
 	}
 
@@ -195,7 +210,7 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 	// container. It's just cleaner to do this here (at the expense of the
 	// operation not being perfectly split).
 
-	if err := unix.Chdir(config.Rootfs); err != nil {
+	if err := unix.Fchdir(int(rootFd.Fd())); err != nil {
 		return &os.PathError{Op: "chdir", Path: config.Rootfs, Err: err}
 	}
 
@@ -210,13 +225,14 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 	if config.NoPivotRoot {
 		err = msMoveRoot(config.Rootfs)
 	} else if config.Namespaces.Contains(configs.NEWNS) {
-		err = pivotRoot(config.Rootfs)
+		err = pivotRoot(rootFd)
 	} else {
 		err = chroot()
 	}
 	if err != nil {
 		return fmt.Errorf("error jailing process inside rootfs: %w", err)
 	}
+	rootFd.Close()
 
 	// Apply root mount propagation flags.
 	// This must be done after pivot_root/chroot because the mount propagation flag is applied
@@ -256,7 +272,7 @@ func finalizeRootfs(config *configs.Config) (err error) {
 		if m.Flags&unix.MS_RDONLY != unix.MS_RDONLY {
 			continue
 		}
-		if m.Device == "tmpfs" || utils.CleanPath(m.Destination) == "/dev" {
+		if m.Device == "tmpfs" || pathrs.LexicallyCleanPath(m.Destination) == "/dev" {
 			if err := remountReadonly(m); err != nil {
 				return err
 			}
@@ -328,12 +344,13 @@ func mountCgroupV1(m mountEntry, c *mountConfig) error {
 			// We just created the tmpfs, and so we can just use filepath.Join
 			// here (not to mention we want to make sure we create the path
 			// inside the tmpfs, so we don't want to resolve symlinks).
-			subsystemPath := filepath.Join(c.root, b.Destination)
 			subsystemName := filepath.Base(b.Destination)
-			if err := pathrs.MkdirAllInRoot(c.root, subsystemPath, 0o755); err != nil {
+			subsystemDir, err := pathrs.MkdirAllInRoot(c.root, b.Destination, 0o755)
+			if err != nil {
 				return err
 			}
-			if err := utils.WithProcfd(c.root, b.Destination, func(dstFd string) error {
+			defer subsystemDir.Close()
+			if err := utils.WithProcfdFile(subsystemDir, func(dstFd string) error {
 				flags := defaultMountFlags
 				if m.Flags&unix.MS_RDONLY != 0 {
 					flags = flags | unix.MS_RDONLY
@@ -361,7 +378,8 @@ func mountCgroupV1(m mountEntry, c *mountConfig) error {
 			// symlink(2) is very dumb, it will just shove the path into
 			// the link and doesn't do any checks or relative path
 			// conversion. Also, don't error out if the cgroup already exists.
-			if err := os.Symlink(mc, filepath.Join(c.root, m.Destination, ss)); err != nil && !os.IsExist(err) {
+			ssPath := filepath.Join(m.Destination, ss)
+			if err := pathrs.SymlinkInRoot(mc, c.root, ssPath); err != nil && !errors.Is(err, os.ErrExist) {
 				return err
 			}
 		}
@@ -425,15 +443,22 @@ func doTmpfsCopyUp(m mountEntry, mountLabel string) (Err error) {
 	}
 	defer tmpDirFile.Close()
 
+	hostRootFd, err := os.OpenFile("/", unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_PATH, 0)
+	if err != nil {
+		return fmt.Errorf("tmpcopyup: open host root: %w", err)
+	}
+	defer hostRootFd.Close()
+
 	// Configure the *host* tmpdir as if it's the container mount. We change
 	// m.dstFile since we are going to mount *on the host*.
 	hostMount := mountEntry{
 		Mount:   m.Mount,
 		dstFile: tmpDirFile,
 	}
-	if err := hostMount.mountPropagate("/", mountLabel); err != nil {
+	if err := hostMount.mountPropagate(hostRootFd, mountLabel); err != nil {
 		return err
 	}
+	hostRootFd.Close()
 	defer func() {
 		if Err != nil {
 			if err := unmount(tmpDir, unix.MNT_DETACH); err != nil {
@@ -502,11 +527,10 @@ func statfsToMountFlags(st unix.Statfs_t) int {
 	return flags
 }
 
-var errRootfsToFile = errors.New("config tries to change rootfs to file")
-
-func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) {
-	unsafePath := utils.StripRoot(rootfs, m.Destination)
-	dstFile, err := pathrs.OpenInRoot(rootfs, unsafePath, unix.O_PATH)
+func (m *mountEntry) createOpenMountpoint(root *os.File) (Err error) {
+	rootfs := root.Name()
+	unsafePath := pathrs.LexicallyStripRoot(rootfs, m.Destination)
+	dstFile, err := pathrs.OpenInRoot(root, unsafePath, unix.O_PATH)
 	defer func() {
 		if dstFile != nil && Err != nil {
 			_ = dstFile.Close()
@@ -542,22 +566,10 @@ func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) {
 			}
 			dstIsFile = !fi.IsDir()
 		}
-
-		// In previous runc versions, we would tolerate nonsense paths with
-		// dangling symlinks as path components. pathrs-lite does not support
-		// this, so instead we have to emulate this behaviour by doing
-		// SecureJoin *purely to get a semi-reasonable path to use* and then we
-		// use pathrs-lite to operate on the path safely.
-		newUnsafePath, err := securejoin.SecureJoin(rootfs, unsafePath)
-		if err != nil {
-			return err
-		}
-		unsafePath = utils.StripRoot(rootfs, newUnsafePath)
-
 		if dstIsFile {
-			dstFile, err = pathrs.CreateInRoot(rootfs, unsafePath, unix.O_CREAT|unix.O_EXCL|unix.O_NOFOLLOW, 0o644)
+			dstFile, err = pathrs.CreateInRoot(root, unsafePath, unix.O_CREAT|unix.O_EXCL|unix.O_NOFOLLOW, 0o644)
 		} else {
-			dstFile, err = pathrs.MkdirAllInRootOpen(rootfs, unsafePath, 0o755)
+			dstFile, err = pathrs.MkdirAllInRoot(root, unsafePath, 0o755)
 		}
 		if err != nil {
 			return fmt.Errorf("make mountpoint %q: %w", m.Destination, err)
@@ -587,7 +599,6 @@ func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) {
 }
 
 func mountToRootfs(c *mountConfig, m mountEntry) error {
-	rootfs := c.root
 	defer func() {
 		if m.dstFile != nil {
 			_ = m.dstFile.Close()
@@ -604,6 +615,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		// has been a "fun" attack scenario in the past.
 		// TODO: This won't be necessary once we switch to libpathrs and we can
 		//       stop all of these symlink-exchange attacks.
+		rootfs := c.root.Name()
 		dest := filepath.Clean(m.Destination)
 		if !pathrs.IsLexicallyInRoot(rootfs, dest) {
 			// Do not use securejoin as it resolves symlinks.
@@ -619,7 +631,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		} else if !fi.IsDir() {
 			return fmt.Errorf("filesystem %q must be mounted on ordinary directory", m.Device)
 		}
-		dstFile, err := pathrs.MkdirAllInRootOpen(rootfs, dest, 0o755)
+		dstFile, err := pathrs.MkdirAllInRoot(c.root, dest, 0o755)
 		if err != nil {
 			return err
 		}
@@ -627,17 +639,17 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		// "proc" and "sys" mounts need special handling (without resolving the
 		// destination) to avoid attacks.
 		m.dstFile = dstFile
-		return m.mountPropagate(rootfs, "")
+		return m.mountPropagate(c.root, "")
 	}
 
 	mountLabel := c.label
-	if err := m.createOpenMountpoint(rootfs); err != nil {
+	if err := m.createOpenMountpoint(c.root); err != nil {
 		return fmt.Errorf("create mountpoint for %s mount: %w", m.Destination, err)
 	}
 
 	switch m.Device {
 	case "mqueue":
-		if err := m.mountPropagate(rootfs, ""); err != nil {
+		if err := m.mountPropagate(c.root, ""); err != nil {
 			return err
 		}
 		return utils.WithProcfdFile(m.dstFile, func(dstFd string) error {
@@ -648,12 +660,12 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		if m.Extensions&configs.EXT_COPYUP == configs.EXT_COPYUP {
 			err = doTmpfsCopyUp(m, mountLabel)
 		} else {
-			err = m.mountPropagate(rootfs, mountLabel)
+			err = m.mountPropagate(c.root, mountLabel)
 		}
 		return err
 	case "bind":
 		// open_tree()-related shenanigans are all handled in mountViaFds.
-		if err := m.mountPropagate(rootfs, mountLabel); err != nil {
+		if err := m.mountPropagate(c.root, mountLabel); err != nil {
 			return err
 		}
 
@@ -775,7 +787,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		}
 		return mountCgroupV1(m, c)
 	default:
-		return m.mountPropagate(rootfs, mountLabel)
+		return m.mountPropagate(c.root, mountLabel)
 	}
 }
 
@@ -890,7 +902,7 @@ func checkProcMount(rootfs, dest string, m mountEntry) error {
 	return fmt.Errorf("%q cannot be mounted because it is inside /proc", dest)
 }
 
-func setupDevSymlinks(rootfs string) error {
+func setupDevSymlinks(rootFd *os.File) error {
 	// In theory, these should be links to /proc/thread-self, but systems
 	// expect these to be /proc/self and this matches how most distributions
 	// work.
@@ -906,11 +918,8 @@ func setupDevSymlinks(rootfs string) error {
 		links = append(links, [2]string{"/proc/kcore", "/dev/core"})
 	}
 	for _, link := range links {
-		var (
-			src = link[0]
-			dst = filepath.Join(rootfs, link[1])
-		)
-		if err := os.Symlink(src, dst); err != nil && !os.IsExist(err) {
+		target, devName := link[0], link[1]
+		if err := pathrs.SymlinkInRoot(target, rootFd, devName); err != nil && !errors.Is(err, os.ErrExist) {
 			return err
 		}
 	}
@@ -950,18 +959,18 @@ func reOpenDevNull() error {
 }
 
 // Create the device nodes in the container.
-func createDevices(config *configs.Config) error {
+func createDevices(rootFd *os.File, config *configs.Config) error {
 	useBindMount := userns.RunningInUserNS() || config.Namespaces.Contains(configs.NEWUSER)
 	for _, node := range config.Devices {
 
 		// The /dev/ptmx device is setup by setupPtmx()
-		if utils.CleanPath(node.Path) == "/dev/ptmx" {
+		if pathrs.LexicallyCleanPath(node.Path) == "/dev/ptmx" {
 			continue
 		}
 
 		// containers running in a user namespace are not allowed to mknod
 		// devices so we can just bind mount it from the host.
-		if err := createDeviceNode(config.Rootfs, node, useBindMount); err != nil {
+		if err := createDeviceNode(rootFd, node, useBindMount); err != nil {
 			return err
 		}
 	}
@@ -981,20 +990,12 @@ func bindMountDeviceNode(destDir *os.File, destName string, node *devices.Device
 }
 
 // Creates the device node in the rootfs of the container.
-func createDeviceNode(rootfs string, node *devices.Device, bind bool) error {
+func createDeviceNode(rootFd *os.File, node *devices.Device, bind bool) error {
 	if node.Path == "" {
 		// The node only exists for cgroup reasons, ignore it here.
 		return nil
 	}
-	destPath, err := securejoin.SecureJoin(rootfs, node.Path)
-	if err != nil {
-		return err
-	}
-	if destPath == rootfs {
-		return fmt.Errorf("%w: mknod over rootfs", errRootfsToFile)
-	}
-	destDirPath, destName := filepath.Split(destPath)
-	destDir, err := pathrs.MkdirAllInRootOpen(rootfs, destDirPath, 0o755)
+	destDir, destName, err := pathrs.MkdirAllParentInRoot(rootFd, node.Path, 0o755)
 	if err != nil {
 		return fmt.Errorf("mkdir parent of device inode %q: %w", node.Path, err)
 	}
@@ -1130,41 +1131,31 @@ func setReadonly() error {
 	return mount("", "/", "", flags, "")
 }
 
-func setupPtmx(config *configs.Config) error {
-	ptmx := filepath.Join(config.Rootfs, "dev/ptmx")
-	if err := os.Remove(ptmx); err != nil && !os.IsNotExist(err) {
+func setupPtmx(rootFd *os.File) error {
+	if err := pathrs.UnlinkInRoot(rootFd, "/dev/ptmx", 0); err != nil && !errors.Is(err, os.ErrNotExist) {
 		return err
 	}
-	if err := os.Symlink("pts/ptmx", ptmx); err != nil {
-		return err
-	}
-	return nil
+	return pathrs.SymlinkInRoot("pts/ptmx", rootFd, "/dev/ptmx")
 }
 
 // pivotRoot will call pivot_root such that rootfs becomes the new root
 // filesystem, and everything else is cleaned up.
-func pivotRoot(rootfs string) error {
+func pivotRoot(root *os.File) error {
 	// While the documentation may claim otherwise, pivot_root(".", ".") is
 	// actually valid. What this results in is / being the new root but
 	// /proc/self/cwd being the old root. Since we can play around with the cwd
 	// with pivot_root this allows us to pivot without creating directories in
 	// the rootfs. Shout-outs to the LXC developers for giving us this idea.
 
-	oldroot, err := unix.Open("/", unix.O_DIRECTORY|unix.O_RDONLY, 0)
+	oldroot, err := unix.Open("/", unix.O_DIRECTORY|unix.O_RDONLY|unix.O_PATH, 0)
 	if err != nil {
 		return &os.PathError{Op: "open", Path: "/", Err: err}
 	}
 	defer unix.Close(oldroot) //nolint: errcheck
 
-	newroot, err := unix.Open(rootfs, unix.O_DIRECTORY|unix.O_RDONLY, 0)
-	if err != nil {
-		return &os.PathError{Op: "open", Path: rootfs, Err: err}
-	}
-	defer unix.Close(newroot) //nolint: errcheck
-
 	// Change to the new root so that the pivot_root actually acts on it.
-	if err := unix.Fchdir(newroot); err != nil {
-		return &os.PathError{Op: "fchdir", Path: "fd " + strconv.Itoa(newroot), Err: err}
+	if err := unix.Fchdir(int(root.Fd())); err != nil {
+		return &os.PathError{Op: "chdir", Path: root.Name(), Err: err}
 	}
 
 	if err := unix.PivotRoot(".", "."); err != nil {
@@ -1406,7 +1397,12 @@ func maskPaths(rootFs string, paths []string, mountLabel string) error {
 					// resolves the underlying inode via procfs and re-opens it through
 					// rootFd, so the resulting fd is anchored to the real path inside the
 					// container rootfs even if path was a /proc/self/fd/N alias.
-					reopened, err := reopenAfterMount(rootFs, dstFh, unix.O_PATH|unix.O_CLOEXEC)
+					rootFd, err := os.OpenFile(rootFs, unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_PATH, 0)
+					if err != nil {
+						return fmt.Errorf("open rootfs handle for masked paths: %w", err)
+					}
+					reopened, err := reopenAfterMount(rootFd, dstFh, unix.O_PATH|unix.O_CLOEXEC)
+					rootFd.Close()
 					if err != nil {
 						return fmt.Errorf("can't reopen shared directory mask: %w", err)
 					}
@@ -1429,16 +1425,17 @@ func maskPaths(rootFs string, paths []string, mountLabel string) error {
 	return nil
 }
 
-func reopenAfterMount(rootfs string, f *os.File, flags int) (_ *os.File, Err error) {
+func reopenAfterMount(rootFd, f *os.File, flags int) (_ *os.File, Err error) {
 	fullPath, err := procfs.ProcSelfFdReadlink(f)
 	if err != nil {
 		return nil, fmt.Errorf("get full path: %w", err)
 	}
+	rootfs := rootFd.Name()
 	if !pathrs.IsLexicallyInRoot(rootfs, fullPath) {
 		return nil, fmt.Errorf("mountpoint %q is outside of rootfs %q", fullPath, rootfs)
 	}
-	unsafePath := utils.StripRoot(rootfs, fullPath)
-	reopened, err := pathrs.OpenInRoot(rootfs, unsafePath, flags)
+	unsafePath := pathrs.LexicallyStripRoot(rootfs, fullPath)
+	reopened, err := pathrs.OpenInRoot(rootFd, unsafePath, flags)
 	if err != nil {
 		return nil, fmt.Errorf("re-open mountpoint %q: %w", unsafePath, err)
 	}
@@ -1468,7 +1465,7 @@ func reopenAfterMount(rootfs string, f *os.File, flags int) (_ *os.File, Err err
 
 // Do the mount operation followed by additional mounts required to take care
 // of propagation flags. This will always be scoped inside the container rootfs.
-func (m *mountEntry) mountPropagate(rootfs string, mountLabel string) error {
+func (m *mountEntry) mountPropagate(rootFd *os.File, mountLabel string) error {
 	var (
 		data  = label.FormatMountLabel(m.Data, mountLabel)
 		flags = m.Flags
@@ -1477,7 +1474,7 @@ func (m *mountEntry) mountPropagate(rootfs string, mountLabel string) error {
 	// operations on it. We need to set up files in "/dev", and other tmpfs
 	// mounts may need to be chmod-ed after mounting. These mounts will be
 	// remounted ro later in finalizeRootfs(), if necessary.
-	if m.Device == "tmpfs" || utils.CleanPath(m.Destination) == "/dev" {
+	if m.Device == "tmpfs" || pathrs.LexicallyCleanPath(m.Destination) == "/dev" {
 		flags &= ^unix.MS_RDONLY
 	}
 
@@ -1494,16 +1491,14 @@ func (m *mountEntry) mountPropagate(rootfs string, mountLabel string) error {
 	//
 	// TODO: Use move_mount(2) on newer kernels so that this is no longer
 	// necessary on modern systems.
-	newDstFile, err := reopenAfterMount(rootfs, m.dstFile, unix.O_PATH)
+	newDstFile, err := reopenAfterMount(rootFd, m.dstFile, unix.O_PATH)
 	if err != nil {
 		return fmt.Errorf("reopen mountpoint after mount: %w", err)
 	}
 	_ = m.dstFile.Close()
 	m.dstFile = newDstFile
 
-	// We have to apply mount propagation flags in a separate WithProcfd() call
-	// because the previous call invalidates the passed procfd -- the mount
-	// target needs to be re-opened.
+	// Apply the propagation flags on the new mount.
 	if err := utils.WithProcfdFile(m.dstFile, func(dstFd string) error {
 		for _, pflag := range m.PropagationFlags {
 			if err := mountViaFds("", nil, m.Destination, dstFd, "", uintptr(pflag), ""); err != nil {

libcontainer/specconv/spec_linux.go

@@ -14,16 +14,16 @@ import (
 
 	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
 	dbus "github.com/godbus/dbus/v5"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
+
 	"github.com/opencontainers/cgroups"
 	devices "github.com/opencontainers/cgroups/devices/config"
+	"github.com/opencontainers/runc/internal/pathrs"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/internal/userns"
 	"github.com/opencontainers/runc/libcontainer/seccomp"
-	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
-	"github.com/opencontainers/runtime-spec/specs-go"
-	"github.com/sirupsen/logrus"
-
-	"golang.org/x/sys/unix"
 )
 
 var (
@@ -746,7 +746,7 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*cgrou
 		if useSystemdCgroup {
 			myCgroupPath = spec.Linux.CgroupsPath
 		} else {
-			myCgroupPath = libcontainerUtils.CleanPath(spec.Linux.CgroupsPath)
+			myCgroupPath = pathrs.LexicallyCleanPath(spec.Linux.CgroupsPath)
 		}
 	}
 

libcontainer/utils/utils.go

@@ -3,11 +3,11 @@ package utils
 import (
 	"encoding/json"
 	"io"
-	"os"
-	"path/filepath"
 	"strings"
 
 	"golang.org/x/sys/unix"
+
+	"github.com/opencontainers/runc/internal/pathrs"
 )
 
 const (
@@ -36,53 +36,6 @@ func WriteJSON(w io.Writer, v interface{}) error {
 	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 {
-	// 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 "../../../../<etc>/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
-}
-
-// StripRoot returns the passed path, stripping the root path if it was
-// (lexicially) inside it. Note that both passed paths will always be treated
-// as absolute, and the returned path will also always be absolute. In
-// addition, the paths are cleaned before stripping the root.
-func StripRoot(root, path string) string {
-	// Make the paths clean and absolute.
-	root, path = CleanPath("/"+root), CleanPath("/"+path)
-	switch {
-	case path == root:
-		path = "/"
-	case root == "/":
-		// do nothing
-	default:
-		path = strings.TrimPrefix(path, root+"/")
-	}
-	return CleanPath("/" + path)
-}
-
 // SearchLabels searches through a list of key=value pairs for a given key,
 // returning its value, and the binary flag telling whether the key exist.
 func SearchLabels(labels []string, key string) (string, bool) {
@@ -113,3 +66,23 @@ func Annotations(labels []string) (bundle string, userAnnotations map[string]str
 	}
 	return bundle, userAnnotations
 }
+
+// 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.
+//
+// Deprecated: This function has been moved to internal/pathrs and this wrapper
+// will be removed in runc 1.5.
+var CleanPath = pathrs.LexicallyCleanPath
+
+// StripRoot returns the passed path, stripping the root path if it was
+// (lexicially) inside it. Note that both passed paths will always be treated
+// as absolute, and the returned path will also always be absolute. In
+// addition, the paths are cleaned before stripping the root.
+//
+// Deprecated: This function has been moved to internal/pathrs and this wrapper
+// will be removed in runc 1.5.
+var StripRoot = pathrs.LexicallyStripRoot

libcontainer/utils/utils_test.go

@@ -70,70 +70,3 @@ func TestWriteJSON(t *testing.T) {
 		t.Errorf("expected to write %s but was %s", expected, b.String())
 	}
 }
-
-func TestCleanPath(t *testing.T) {
-	path := CleanPath("")
-	if path != "" {
-		t.Errorf("expected to receive empty string and received %s", path)
-	}
-
-	path = CleanPath("rootfs")
-	if path != "rootfs" {
-		t.Errorf("expected to receive 'rootfs' and received %s", path)
-	}
-
-	path = CleanPath("../../../var")
-	if path != "var" {
-		t.Errorf("expected to receive 'var' and received %s", path)
-	}
-
-	path = CleanPath("/../../../var")
-	if path != "/var" {
-		t.Errorf("expected to receive '/var' and received %s", path)
-	}
-
-	path = CleanPath("/foo/bar/")
-	if path != "/foo/bar" {
-		t.Errorf("expected to receive '/foo/bar' and received %s", path)
-	}
-
-	path = CleanPath("/foo/bar/../")
-	if path != "/foo" {
-		t.Errorf("expected to receive '/foo' and received %s", path)
-	}
-}
-
-func TestStripRoot(t *testing.T) {
-	for _, test := range []struct {
-		root, path, out string
-	}{
-		// Works with multiple components.
-		{"/a/b", "/a/b/c", "/c"},
-		{"/hello/world", "/hello/world/the/quick-brown/fox", "/the/quick-brown/fox"},
-		// '/' must be a no-op.
-		{"/", "/a/b/c", "/a/b/c"},
-		// Must be the correct order.
-		{"/a/b", "/a/c/b", "/a/c/b"},
-		// Must be at start.
-		{"/abc/def", "/foo/abc/def/bar", "/foo/abc/def/bar"},
-		// Must be a lexical parent.
-		{"/foo/bar", "/foo/barSAMECOMPONENT", "/foo/barSAMECOMPONENT"},
-		// Must only strip the root once.
-		{"/foo/bar", "/foo/bar/foo/bar/baz", "/foo/bar/baz"},
-		// Deal with .. in a fairly sane way.
-		{"/foo/bar", "/foo/bar/../baz", "/foo/baz"},
-		{"/foo/bar", "../../../../../../foo/bar/baz", "/baz"},
-		{"/foo/bar", "/../../../../../../foo/bar/baz", "/baz"},
-		{"/foo/bar/../baz", "/foo/baz/bar", "/bar"},
-		{"/foo/bar/../baz", "/foo/baz/../bar/../baz/./foo", "/foo"},
-		// All paths are made absolute before stripping.
-		{"foo/bar", "/foo/bar/baz/bee", "/baz/bee"},
-		{"/foo/bar", "foo/bar/baz/beef", "/baz/beef"},
-		{"foo/bar", "foo/bar/baz/beets", "/baz/beets"},
-	} {
-		got := StripRoot(test.root, test.path)
-		if got != test.out {
-			t.Errorf("StripRoot(%q, %q) -- got %q, expected %q", test.root, test.path, got, test.out)
-		}
-	}
-}

libcontainer/utils/utils_unix.go

@@ -13,9 +13,10 @@ import (
 	_ "unsafe" // for go:linkname
 
 	securejoin "github.com/cyphar/filepath-securejoin"
-	"github.com/opencontainers/runc/internal/pathrs"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
+
+	"github.com/opencontainers/runc/internal/pathrs"
 )
 
 var (
@@ -150,9 +151,12 @@ func NewSockPair(name string) (parent, child *os.File, err error) {
 // through the passed fdpath should be safe. Do not access this path through
 // the original path strings, and do not attempt to use the pathname outside of
 // the passed closure (the file handle will be freed once the closure returns).
+//
+// Deprecated: This function is an internal implementation detail of runc and
+// is no longer used. It will be removed in runc 1.5.
 func WithProcfd(root, unsafePath string, fn func(procfd string) error) error {
 	// Remove the root then forcefully resolve inside the root.
-	unsafePath = StripRoot(root, unsafePath)
+	unsafePath = pathrs.LexicallyStripRoot(root, unsafePath)
 	fullPath, err := securejoin.SecureJoin(root, unsafePath)
 	if err != nil {
 		return fmt.Errorf("resolving path inside rootfs failed: %w", err)
@@ -179,10 +183,15 @@ func WithProcfd(root, unsafePath string, fn func(procfd string) error) error {
 	return fn(procfd)
 }
 
-// WithProcfdFile is a very minimal wrapper around [ProcThreadSelfFd], intended
-// to make migrating from [WithProcfd] and [WithProcfdPath] usage easier. The
+// WithProcfdFile is a very minimal wrapper around [ProcThreadSelfFd]. The
 // caller is responsible for making sure that the provided file handle is
 // actually safe to operate on.
+//
+// NOTE: THIS FUNCTION IS INTERNAL TO RUNC, DO NOT USE IT.
+//
+// TODO: Migrate the mount logic towards a more move_mount(2)-friendly design
+// where this is kind of /proc/self/... tomfoolery is only done in a fallback
+// path for old kernels.
 func WithProcfdFile(file *os.File, fn func(procfd string) error) error {
 	fdpath, closer := ProcThreadSelfFd(file.Fd())
 	defer closer()

tests/integration/mounts.bats

@@ -127,6 +127,42 @@ function test_mount_order() {
 	[[ "$output" == *"a/x"* ]] # the final "file" was from a/x.
 }
 
+# This needs to be placed at the top of the bats file to work around
+# a shellcheck bug. See <https://github.com/koalaman/shellcheck/issues/2873>.
+test_mount_target() {
+	src="$1"
+	dst="$2"
+	real_dst="${3:-$dst}"
+
+	echo "== $src -> $dst (=> $real_dst) =="
+
+	old_config="$(mktemp ./config.json.bak.XXXXXX)"
+	cp ./config.json "$old_config"
+
+	update_config '.mounts += [{
+		source: "'"$src"'",
+		destination: "'"$dst"'",
+		options: ["bind"]
+	}]'
+
+	# Make sure the target path is at the right spot and is actually a
+	# bind-mount of the correct inode.
+	update_config '.process.args = ["stat", "-c", "%n %d:%i", "--", "'"$real_dst"'"]'
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "$output" == "$real_dst $(stat -c "%d:%i" -- "$src")" ]]
+
+	# Make sure there is a mount entry for the target path.
+	# shellcheck disable=SC2016
+	update_config '.process.args = ["awk", "-F", "PATH='"$real_dst"'", "$2 == PATH", "/proc/self/mounts"]'
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "$output" == *"$real_dst"* ]]
+
+	# Switch back the old config so this function can be called multiple times.
+	mv "$old_config" "./config.json"
+}
+
 # https://github.com/opencontainers/runc/issues/3991
 @test "runc run [tmpcopyup]" {
 	mkdir -p rootfs/dir1/dir2
@@ -343,3 +379,25 @@ function test_mount_order() {
 @test "runc run [mount order, container idmap source] (userns)" {
 	test_mount_order userns,idmap
 }
+
+@test "runc run [bind mount through a dangling symlink component]" {
+	rm -rf rootfs/etc/hosts
+	ln -s /tmp/foo/bar rootfs/jump
+	ln -s /jump/baz/hosts rootfs/etc/hosts
+
+	rm -rf rootfs/tmp/foo
+	test_mount_target ./config.json /etc/hosts /tmp/foo/bar/baz/hosts
+}
+
+@test "runc run [bind mount through a trailing dangling symlink]" {
+	rm -rf rootfs/etc/hosts
+	ln -s /tmp/hosts rootfs/etc/hosts
+
+	# File.
+	rm -rf rootfs/tmp/hosts
+	test_mount_target ./config.json /etc/hosts /tmp/hosts
+
+	# Directory.
+	rm -rf rootfs/tmp/hosts
+	test_mount_target . /etc/hosts /tmp/hosts
+}