From a4afb37ba760e6ba0fc3d302279b4a2db850f803 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 18 Mar 2026 17:17:27 -0700 Subject: [PATCH 01/10] libct: mountCgroupV1: address TODO Indeed, it does not make sense to prepend c.root once we started using MkdirAllInRoot in commit 63c29081. Signed-off-by: Kir Kolyshkin (cherry picked from commit 60352524d346721b6a8640772e9b6e23e01a6ffa) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 7aaa70d10..938a8078e 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -336,10 +336,8 @@ 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). - // TODO: Why not just use b.Destination (c.root is the root here)? - subsystemPath := filepath.Join(c.root, b.Destination) subsystemName := filepath.Base(b.Destination) - subsystemDir, err := pathrs.MkdirAllInRoot(c.root, subsystemPath, 0o755) + subsystemDir, err := pathrs.MkdirAllInRoot(c.root, b.Destination, 0o755) if err != nil { return err } From 63857e5d2014ec9a7a3191f4dbd41ff877a780a1 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 20 Mar 2026 14:29:23 -0700 Subject: [PATCH 02/10] libct: minor refactor in mountToRootfs No change in functionality, just a preparation for the next patch. Signed-off-by: Kir Kolyshkin (cherry picked from commit 78b80677f6713372207a2d65b0bd22d17f432f22) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 938a8078e..c9e9e96ee 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -582,7 +582,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() @@ -599,6 +598,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 dest := filepath.Clean(m.Destination) if !pathrs.IsLexicallyInRoot(rootfs, dest) { // Do not use securejoin as it resolves symlinks. @@ -614,7 +614,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.MkdirAllInRoot(rootfs, dest, 0o755) + dstFile, err := pathrs.MkdirAllInRoot(c.root, dest, 0o755) if err != nil { return err } @@ -622,17 +622,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 { @@ -643,12 +643,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 } @@ -770,7 +770,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error { } return mountCgroupV1(m, c) default: - return m.mountPropagate(rootfs, mountLabel) + return m.mountPropagate(c.root, mountLabel) } } From aa769e7dcc09a907952f6b2f5be0a8191c1584ed Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 18 Mar 2026 17:21:55 -0700 Subject: [PATCH 03/10] Pre-open container root directory A lot of filesystem-related stuff happens inside the container root directory, and we have used its name before. It makes sense to pre-open it and use a *os.File handle instead. Function names in internal/pathrs are kept as is for simplicity (and it is an internal package), but they now accept root as *os.File. Signed-off-by: Kir Kolyshkin (cherry picked from commit 28cb3218873f7b073fba69c74663c9c1ffdfbe6b) Signed-off-by: lifubang --- internal/pathrs/mkdirall.go | 4 +- internal/pathrs/mkdirall_pathrslite.go | 15 ++----- internal/pathrs/root_pathrslite.go | 10 ++--- libcontainer/criu_linux.go | 10 ++++- libcontainer/rootfs_linux.go | 55 +++++++++++++++++--------- 5 files changed, 56 insertions(+), 38 deletions(-) diff --git a/internal/pathrs/mkdirall.go b/internal/pathrs/mkdirall.go index 3a896f484..81c9a022c 100644 --- a/internal/pathrs/mkdirall.go +++ b/internal/pathrs/mkdirall.go @@ -31,12 +31,12 @@ import ( // 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, unsafePath string, mode os.FileMode) (*os.File, string, error) { +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, unsafePath) + unsafePath, err := hallucinateUnsafePath(root.Name(), unsafePath) if err != nil { return nil, "", fmt.Errorf("failed to construct hallucinated target path: %w", err) } diff --git a/internal/pathrs/mkdirall_pathrslite.go b/internal/pathrs/mkdirall_pathrslite.go index c2578e051..0a6f25ebd 100644 --- a/internal/pathrs/mkdirall_pathrslite.go +++ b/internal/pathrs/mkdirall_pathrslite.go @@ -24,12 +24,11 @@ import ( "github.com/cyphar/filepath-securejoin/pathrs-lite" "github.com/sirupsen/logrus" - "golang.org/x/sys/unix" ) // MkdirAllInRoot attempts to make // -// path, _ := securejoin.SecureJoin(root, unsafePath) +// path, _ := securejoin.SecureJoin(root.Name(), unsafePath) // os.MkdirAll(path, mode) // os.Open(path) // @@ -48,8 +47,8 @@ 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 MkdirAllInRoot(root, unsafePath string, mode os.FileMode) (*os.File, error) { - unsafePath, err := hallucinateUnsafePath(root, unsafePath) +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) } @@ -67,13 +66,7 @@ func MkdirAllInRoot(root, unsafePath string, mode os.FileMode) (*os.File, error) 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) }) } diff --git a/internal/pathrs/root_pathrslite.go b/internal/pathrs/root_pathrslite.go index 51db77440..0ddabf804 100644 --- a/internal/pathrs/root_pathrslite.go +++ b/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,7 +47,7 @@ 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) { +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 @@ -63,5 +63,5 @@ 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 } diff --git a/libcontainer/criu_linux.go b/libcontainer/criu_linux.go index 322e43957..736743833 100644 --- a/libcontainer/criu_linux.go +++ b/libcontainer/criu_linux.go @@ -540,10 +540,16 @@ 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 { - mntFile, err := pathrs.OpenInRoot(c.config.Rootfs, u, unix.O_PATH) + 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 @@ -577,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 { diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index c9e9e96ee..949798133 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -35,7 +35,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 @@ -160,8 +160,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, @@ -175,7 +184,7 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) { setupDev := needsSetupDev(config) if setupDev { - if err := createDevices(config); err != nil { + if err := createDevices(config, rootFd); err != nil { return fmt.Errorf("error creating device nodes: %w", err) } if err := setupPtmx(config); err != nil { @@ -225,6 +234,7 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) { 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 @@ -370,7 +380,7 @@ 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 && !errors.Is(err, os.ErrExist) { + if err := os.Symlink(mc, filepath.Join(c.root.Name(), m.Destination, ss)); err != nil && !errors.Is(err, os.ErrExist) { return err } } @@ -434,15 +444,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 { @@ -511,9 +528,10 @@ func statfsToMountFlags(st unix.Statfs_t) int { return flags } -func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) { +func (m *mountEntry) createOpenMountpoint(root *os.File) (Err error) { + rootfs := root.Name() unsafePath := pathrs.LexicallyStripRoot(rootfs, m.Destination) - dstFile, err := pathrs.OpenInRoot(rootfs, unsafePath, unix.O_PATH) + dstFile, err := pathrs.OpenInRoot(root, unsafePath, unix.O_PATH) defer func() { if dstFile != nil && Err != nil { _ = dstFile.Close() @@ -550,9 +568,9 @@ func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) { dstIsFile = !fi.IsDir() } 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.MkdirAllInRoot(rootfs, unsafePath, 0o755) + dstFile, err = pathrs.MkdirAllInRoot(root, unsafePath, 0o755) } if err != nil { return fmt.Errorf("make mountpoint %q: %w", m.Destination, err) @@ -598,7 +616,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 + rootfs := c.root.Name() dest := filepath.Clean(m.Destination) if !pathrs.IsLexicallyInRoot(rootfs, dest) { // Do not use securejoin as it resolves symlinks. @@ -941,7 +959,7 @@ func reOpenDevNull() error { } // Create the device nodes in the container. -func createDevices(config *configs.Config) error { +func createDevices(config *configs.Config, rootFd *os.File) error { useBindMount := userns.RunningInUserNS() || config.Namespaces.Contains(configs.NEWUSER) for _, node := range config.Devices { @@ -952,7 +970,7 @@ func createDevices(config *configs.Config) error { // 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 } } @@ -972,12 +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 } - destDir, destName, err := pathrs.MkdirAllParentInRoot(rootfs, node.Path, 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) } @@ -1370,16 +1388,17 @@ func maskPaths(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 := pathrs.LexicallyStripRoot(rootfs, fullPath) - reopened, err := pathrs.OpenInRoot(rootfs, unsafePath, flags) + reopened, err := pathrs.OpenInRoot(rootFd, unsafePath, flags) if err != nil { return nil, fmt.Errorf("re-open mountpoint %q: %w", unsafePath, err) } @@ -1409,7 +1428,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, mountLabel string) error { +func (m *mountEntry) mountPropagate(rootFd *os.File, mountLabel string) error { var ( data = label.FormatMountLabel(m.Data, mountLabel) flags = m.Flags @@ -1435,7 +1454,7 @@ func (m *mountEntry) mountPropagate(rootfs, 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) } From 522b4c804e588998f513e0dd71bf768ff5dc7160 Mon Sep 17 00:00:00 2001 From: lfbzhm Date: Fri, 27 Mar 2026 13:27:43 -0700 Subject: [PATCH 04/10] libct: use preopened rootfs more This uses preopened rootfs in Chdir and pivotRoot. While at it, add O_PATH when opening oldroot in pivotRoot. Co-authored-by: Kir Kolyshkin Signed-off-by: lfbzhm Signed-off-by: Kir Kolyshkin (cherry picked from commit 5b094ed1ac281d0fd218590d02fa0714cc6953ba) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 949798133..0c8c5836d 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -212,7 +212,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} } @@ -227,7 +227,7 @@ 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() } @@ -1144,28 +1144,22 @@ func setupPtmx(config *configs.Config) error { // 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 := linux.Open("/", unix.O_DIRECTORY|unix.O_RDONLY, 0) + oldroot, err := linux.Open("/", unix.O_DIRECTORY|unix.O_RDONLY|unix.O_PATH, 0) if err != nil { return err } defer unix.Close(oldroot) - newroot, err := linux.Open(rootfs, unix.O_DIRECTORY|unix.O_RDONLY, 0) - if err != nil { - return err - } - defer unix.Close(newroot) - // 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 { From b9e7a27a0117904f607d936a38db04d2271630c3 Mon Sep 17 00:00:00 2001 From: lifubang Date: Tue, 12 May 2026 03:26:27 +0000 Subject: [PATCH 05/10] libct: skip mount for duplicate masked paths Co-authored-by: Davanum Srinivas Refactored-by: lifubang Signed-off-by: lifubang (cherry picked from commit abf70bab6344e1627850d038740245bd9e2787e4) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 9 +++++++++ tests/integration/mask.bats | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 0c8c5836d..9e882a592 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1348,6 +1348,7 @@ func maskPaths(paths []string, mountLabel string) error { procSelfFd, closer := utils.ProcThreadSelf("fd/") defer closer() + maskedPaths := make(map[string]struct{}) for _, path := range paths { // Open the target path; skip if it doesn't exist. dstFh, err := os.OpenFile(path, unix.O_PATH|unix.O_CLOEXEC, 0) @@ -1362,6 +1363,14 @@ func maskPaths(paths []string, mountLabel string) error { dstFh.Close() return fmt.Errorf("can't mask path %q: %w", path, err) } + // skip duplicate masked paths. + cleanPath := pathrs.LexicallyCleanPath(path) + if _, ok := maskedPaths[cleanPath]; ok { + dstFh.Close() + continue + } + maskedPaths[cleanPath] = struct{}{} + var dstType string if st.IsDir() { // Destination is a directory: bind mount a ro tmpfs over it. diff --git a/tests/integration/mask.bats b/tests/integration/mask.bats index 5783332ea..6cc9339df 100644 --- a/tests/integration/mask.bats +++ b/tests/integration/mask.bats @@ -55,6 +55,20 @@ function teardown() { [[ "${output}" == *"Operation not permitted"* ]] } +@test "mask paths [duplicate paths]" { + update_config '(.. | select(.maskedPaths? != null)) .maskedPaths += ["/testdir", "/testfile"]' + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + runc exec test_busybox sh -c "mount | grep /testdir -c" + [ "$status" -eq 0 ] + [[ "${output}" == "1" ]] + + runc exec test_busybox sh -c "mount | grep /testfile -c" + [ "$status" -eq 0 ] + [[ "${output}" == "1" ]] +} + @test "mask paths [prohibit symlink /proc]" { ln -s /symlink rootfs/proc runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox From abc399150b87015bbbb8e242885d6233b8677b1e Mon Sep 17 00:00:00 2001 From: lifubang Date: Tue, 12 May 2026 08:54:29 +0000 Subject: [PATCH 06/10] libct: enforce strict tmpfs limits for masked paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, masked directories (e.g., /proc/acpi, /proc/scsi) were mounted as read-only tmpfs without explicit size or inode limits. Although these mounts are meant to be empty and unwritable, the lack of resource constraints means that—should an attacker bypass the read-only protection (e.g., via container escape, mount namespace manipulation, or a kernel vulnerability)—the tmpfs could consume up to 50% of system memory by default (the kernel's default tmpfs limit). To mitigate this risk in high-density container environments and adhere to the principle of least privilege, we now explicitly set: - nr_blocks=1 (sufficient for at most one block size) - nr_inodes=1 (sufficient for at most one inode) Ref: https://man7.org/linux/man-pages/man5/tmpfs.5.html These limits ensure that even if compromised, kernel memory usage remains strictly bounded and negligible. This change aligns with best practices used by other container runtimes and strengthens defense-in-depth for sensitive masked paths. Co-authored-by: Davanum Srinivas Refactored-by: lifubang Signed-off-by: lifubang (cherry picked from commit e57a7a4c8f76da8bbc299bc8beb14cd1cba741e0) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 9e882a592..db5d015ef 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1375,7 +1375,7 @@ func maskPaths(paths []string, mountLabel string) error { if st.IsDir() { // Destination is a directory: bind mount a ro tmpfs over it. dstType = "dir" - err = mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("", mountLabel)) + err = mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("nr_blocks=1,nr_inodes=1", mountLabel)) } else { // Destination is a file: mount it to /dev/null. dstType = "path" From 8dbcab66d1d16d718065f021cdb95eb2078b7405 Mon Sep 17 00:00:00 2001 From: lifubang Date: Tue, 12 May 2026 04:08:05 +0000 Subject: [PATCH 07/10] libct: reuse tmpfs for directory masks Kubernetes may add one sysfs thermal_throttle entry per CPU to maskedPaths. On large Intel systems this can produce many directory masks for a single container. runc currently handles each directory mask with a separate read-only tmpfs mount, and therefore a separate tmpfs superblock. On Linux 4.18/RHEL 8 kernels, creating and tearing down many tmpfs superblocks can contend on the global shrinker_rwsem when containers start or stop concurrently. Use one read-only tmpfs for directory masks and bind-mount it over the remaining directory targets. The first non-procfs-fd directory mount is reopened through the container root fd before it is reused. File masks still bind /dev/null, and procfs fd targets keep the existing one-tmpfs-per-target behaviour because they are fd aliases rather than stable rootfs paths. If the bind-mount of the shared source fails (e.g. due to kernel restrictions), fall back to individual tmpfs mounts for all remaining directories. Tmpfs mounts use nr_blocks=1,nr_inodes=1 to minimise kernel resource usage. The bind mounts do not create additional tmpfs superblocks. They also retain the read-only mount flag inherited from the source vfsmount, so the masking semantics remain unchanged. xref: kubernetes/kubernetes#138512 xref: kubernetes/kubernetes#138388 xref: kubernetes/kubernetes#131018 Co-authored-by: Davanum Srinivas Refactored-by: lifubang Signed-off-by: lifubang (cherry picked from commit c046c9b973defb8c4617a8deb01696902c227779) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 40 ++++++++++++++++++++++++++--- libcontainer/standard_init_linux.go | 12 +++++++-- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index db5d015ef..90c851b2e 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -419,7 +419,7 @@ func mountCgroupV2(m mountEntry, c *mountConfig) error { // Mask `/sys/fs/cgroup` to ensure it is read-only, even when `/sys` is mounted // with `rbind,ro` (`runc spec --rootless` produces `rbind,ro` for `/sys`). err = utils.WithProcfdFile(m.dstFile, func(procfd string) error { - return maskPaths([]string{procfd}, c.label) + return maskPaths(c.root, []string{procfd}, c.label) }) } return err @@ -1335,7 +1335,7 @@ func verifyDevNull(f *os.File) error { // mounts ( proc/kcore ). // For files, maskPath bind mounts /dev/null over the top of the specified path. // For directories, maskPath mounts read-only tmpfs over the top of the specified path. -func maskPaths(paths []string, mountLabel string) error { +func maskPaths(rootFd *os.File, paths []string, mountLabel string) error { devNull, err := os.OpenFile("/dev/null", unix.O_PATH, 0) if err != nil { return fmt.Errorf("can't mask paths: %w", err) @@ -1348,6 +1348,16 @@ func maskPaths(paths []string, mountLabel string) error { procSelfFd, closer := utils.ProcThreadSelf("fd/") defer closer() + var ( + sharedMaskFile *os.File + sharedMaskSrc *mountSource + bindFailed bool + ) + defer func() { + if sharedMaskFile != nil { + _ = sharedMaskFile.Close() + } + }() maskedPaths := make(map[string]struct{}) for _, path := range paths { // Open the target path; skip if it doesn't exist. @@ -1375,7 +1385,31 @@ func maskPaths(paths []string, mountLabel string) error { if st.IsDir() { // Destination is a directory: bind mount a ro tmpfs over it. dstType = "dir" - err = mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("nr_blocks=1,nr_inodes=1", mountLabel)) + if !bindFailed && sharedMaskSrc != nil { + dstFd := filepath.Join(procSelfFd, strconv.Itoa(int(dstFh.Fd()))) + err = mountViaFds("", sharedMaskSrc, path, dstFd, "", unix.MS_BIND, "") + if err != nil { + // A bind-mount inherits MNT_READONLY from the source vfsmount, + // but if it fails fall back to individual tmpfs mounts. + bindFailed = true + logrus.WithError(err).Warn("maskPaths: shared tmpfs bind-mount failed, falling back to per-directory tmpfs") + } + } + if bindFailed || sharedMaskSrc == nil { + err = mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("nr_blocks=1,nr_inodes=1", mountLabel)) + if err == nil && !bindFailed && sharedMaskSrc == nil { + // Establish this mount as the reusable shared source. reopenAfterMount + // 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(rootFd, dstFh, unix.O_PATH|unix.O_CLOEXEC) + if err != nil { + return fmt.Errorf("can't reopen shared directory mask: %w", err) + } + sharedMaskFile = reopened + sharedMaskSrc = &mountSource{Type: mountSourcePlain, file: sharedMaskFile} + } + } } else { // Destination is a file: mount it to /dev/null. dstType = "path" diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 570472bd4..116e659a8 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -142,8 +142,16 @@ func (l *linuxStandardInit) Init() error { } } - if err := maskPaths(l.config.Config.MaskPaths, l.config.Config.MountLabel); err != nil { - return err + if len(l.config.Config.MaskPaths) > 0 { + rootFd, err := os.OpenFile("/", unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_PATH, 0) + if err != nil { + return fmt.Errorf("open rootfs handle for masked paths: %w", err) + } + err = maskPaths(rootFd, l.config.Config.MaskPaths, l.config.Config.MountLabel) + rootFd.Close() + if err != nil { + return err + } } pdeath, err := system.GetParentDeathSignal() if err != nil { From e454170167f8c6f6eb61457abfb3f3ce8086a450 Mon Sep 17 00:00:00 2001 From: lifubang Date: Tue, 12 May 2026 04:17:27 +0000 Subject: [PATCH 08/10] integration: reuse tmpfs for directory masks Co-authored-by: Davanum Srinivas Signed-off-by: lifubang (cherry picked from commit 124772f3545b62f24ea1ea6a9e90c9d560f625c1) Signed-off-by: lifubang --- tests/integration/mask.bats | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/integration/mask.bats b/tests/integration/mask.bats index 6cc9339df..782edbb30 100644 --- a/tests/integration/mask.bats +++ b/tests/integration/mask.bats @@ -6,7 +6,7 @@ function setup() { setup_busybox # Create fake rootfs. - mkdir rootfs/testdir + mkdir rootfs/testdir rootfs/testdir2 rootfs/testdir3 echo "Forbidden information!" >rootfs/testfile # add extra masked paths @@ -87,3 +87,33 @@ function teardown() { # so we merely check that it fails, and do not check the exact error # message like for /proc above. } + +@test "mask paths [directories share tmpfs]" { + update_config '(.. | select(.maskedPaths? != null)) .maskedPaths += ["/testdir2", "/testdir3"]' + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + # shellcheck disable=SC2016 + runc exec test_busybox sh -euc ' + set -- $(stat -c %d /testdir /testdir2 /testdir3) + [ "$1" = "$2" ] + [ "$2" = "$3" ] + ' + [ "$status" -eq 0 ] + + runc exec test_busybox touch /testdir2/foo + [ "$status" -eq 1 ] + [[ "${output}" == *"Read-only file system"* ]] +} + +@test "mask paths [directory with read-only rootfs]" { + update_config '(.. | select(.maskedPaths? != null)) .maskedPaths += ["/testdir2", "/testdir3"]' + update_config '.root.readonly = true' + + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + runc exec test_busybox ls /testdir + [ "$status" -eq 0 ] + [ -z "$output" ] +} From 81d66c66f99e60bb722867a9af6dad64695031ed Mon Sep 17 00:00:00 2001 From: lifubang Date: Sat, 16 May 2026 04:12:44 +0000 Subject: [PATCH 09/10] libct: optimize maskPaths for single-directory case This is a follow-up to #5275. That change reused a single tmpfs mount to mask multiple directories, which is efficient when masking more than one path. However, it introduced unnecessary overhead when only one directory is masked. This commit restores the original behavior for the single-path case while preserving shared tmpfs logic for multiple paths. Signed-off-by: lifubang (cherry picked from commit e7e2f00248a2700827f1fbc54e8767a53ad11117) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 90c851b2e..f4f930118 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -419,7 +419,7 @@ func mountCgroupV2(m mountEntry, c *mountConfig) error { // Mask `/sys/fs/cgroup` to ensure it is read-only, even when `/sys` is mounted // with `rbind,ro` (`runc spec --rootless` produces `rbind,ro` for `/sys`). err = utils.WithProcfdFile(m.dstFile, func(procfd string) error { - return maskPaths(c.root, []string{procfd}, c.label) + return maskDir(procfd, c.label) }) } return err @@ -1330,6 +1330,11 @@ func verifyDevNull(f *os.File) error { }) } +// maskDir mounts a read-only tmpfs on top of the specified path. +func maskDir(path, mountLabel string) error { + return mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("nr_blocks=1,nr_inodes=1", mountLabel)) +} + // maskPaths masks the top of the specified paths inside a container to avoid // security issues from processes reading information from non-namespace aware // mounts ( proc/kcore ). @@ -1396,7 +1401,7 @@ func maskPaths(rootFd *os.File, paths []string, mountLabel string) error { } } if bindFailed || sharedMaskSrc == nil { - err = mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("nr_blocks=1,nr_inodes=1", mountLabel)) + err = maskDir(path, mountLabel) if err == nil && !bindFailed && sharedMaskSrc == nil { // Establish this mount as the reusable shared source. reopenAfterMount // resolves the underlying inode via procfs and re-opens it through From 0d9be8021fc7889dc16f60a857976dfedc47aaf2 Mon Sep 17 00:00:00 2001 From: lifubang Date: Sat, 16 May 2026 04:40:37 +0000 Subject: [PATCH 10/10] libct: close rootFd ASAP in maskPaths Close the root file descriptor immediately after use in maskPaths to reduce the window during which an attacker could potentially exploit an open fd to access or manipulate the root filesystem. This follows the principle of least privilege and mitigates risks in compromised or malicious container scenarios. Co-authored-by: Kir Kolyshkin Signed-off-by: lifubang (cherry picked from commit b88635e57ec51c83a4b3d2ef281e3cfb692e30ca) Signed-off-by: lifubang --- libcontainer/rootfs_linux.go | 10 +++++++++- libcontainer/standard_init_linux.go | 13 +++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index f4f930118..4eb1cedaf 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1340,7 +1340,10 @@ func maskDir(path, mountLabel string) error { // mounts ( proc/kcore ). // For files, maskPath bind mounts /dev/null over the top of the specified path. // For directories, maskPath mounts read-only tmpfs over the top of the specified path. -func maskPaths(rootFd *os.File, paths []string, mountLabel string) error { +func maskPaths(rootFs string, paths []string, mountLabel string) error { + if len(paths) == 0 { + return nil + } devNull, err := os.OpenFile("/dev/null", unix.O_PATH, 0) if err != nil { return fmt.Errorf("can't mask paths: %w", err) @@ -1407,7 +1410,12 @@ func maskPaths(rootFd *os.File, 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. + 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) } diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 116e659a8..933c1de34 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -142,17 +142,10 @@ func (l *linuxStandardInit) Init() error { } } - if len(l.config.Config.MaskPaths) > 0 { - rootFd, err := os.OpenFile("/", unix.O_DIRECTORY|unix.O_CLOEXEC|unix.O_PATH, 0) - if err != nil { - return fmt.Errorf("open rootfs handle for masked paths: %w", err) - } - err = maskPaths(rootFd, l.config.Config.MaskPaths, l.config.Config.MountLabel) - rootFd.Close() - if err != nil { - return err - } + if err := maskPaths("/", l.config.Config.MaskPaths, l.config.Config.MountLabel); err != nil { + return err } + pdeath, err := system.GetParentDeathSignal() if err != nil { return fmt.Errorf("can't get pdeath signal: %w", err)