libct: switch final WithProcfd users to WithProcfdFile

This probably should've been done as part of commit d40b343
("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 <[email protected]>

Author: cyphar

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
-}

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.

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 {

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()