From 9dbd37e06f8124be6e496308e1735d18f27bb730 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sun, 9 Nov 2025 02:03:42 +1100 Subject: [PATCH] libct: switch final WithProcfd users to WithProcfdFile This probably should've been done as part of commit d40b3439a961 ("rootfs: switch to fd-based handling of mountpoint targets") but it seems I missed them when doing the rest of the conversions. This also lets us remove utils.WithProcfd entirely, as well as pathrs.MkdirAllInRoot. Unfortunately, WithProcfd was exposed in the externally-importable "libcontainer/utils" package and so we need to have a deprecation notice to remove it in runc 1.5. Signed-off-by: Aleksa Sarai --- internal/pathrs/mkdirall_pathrslite.go | 10 ---------- libcontainer/criu_linux.go | 15 +++++++++++---- libcontainer/rootfs_linux.go | 11 ++++++----- libcontainer/utils/utils_unix.go | 12 ++++++++++-- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/internal/pathrs/mkdirall_pathrslite.go b/internal/pathrs/mkdirall_pathrslite.go index a9a0157c6..3fd476fbf 100644 --- a/internal/pathrs/mkdirall_pathrslite.go +++ b/internal/pathrs/mkdirall_pathrslite.go @@ -87,13 +87,3 @@ func MkdirAllInRootOpen(root, unsafePath string, mode os.FileMode) (*os.File, er return pathrs.MkdirAllHandle(rootDir, 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 -} diff --git a/libcontainer/criu_linux.go b/libcontainer/criu_linux.go index 04e0d5f64..8abda7354 100644 --- a/libcontainer/criu_linux.go +++ b/libcontainer/criu_linux.go @@ -25,6 +25,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" ) @@ -555,16 +556,22 @@ func (c *Container) prepareCriuRestoreMounts(mounts []*configs.Mount) error { 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(c.config.Rootfs, 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. diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index f6f58987f..05a7807e6 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -329,12 +329,15 @@ 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) - if err := pathrs.MkdirAllInRoot(c.root, subsystemPath, 0o755); err != nil { + subsystemDir, err := pathrs.MkdirAllInRootOpen(c.root, subsystemPath, 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 @@ -1456,9 +1459,7 @@ func (m *mountEntry) mountPropagate(rootfs string, mountLabel string) error { _ = 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 { diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index 37ec59939..af5689e9a 100644 --- a/libcontainer/utils/utils_unix.go +++ b/libcontainer/utils/utils_unix.go @@ -152,6 +152,9 @@ 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 = pathrs.LexicallyStripRoot(root, unsafePath) @@ -181,10 +184,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()