Allow update-ldcache hook to work when pivot-root is not supported

cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go

@@ -140,25 +140,11 @@ func updateLdCacheHandler() {
 // It is invoked from a reexec'd handler and provides namespace isolation for
 // the operations performed by this hook. At the point where this is invoked,
 // we are in a new mount namespace that is cloned from the parent.
-//
-// args[0] is the reexec initializer function name
-// args[1] is the path of the ldconfig binary on the host
-// args[2] is the container root directory
-// The remaining args are folders where soname symlinks need to be created.
 func updateLdCache(args []string) error {
-	if len(args) < 3 {
-		return fmt.Errorf("incorrect arguments: %v", args)
-	}
-	hostLdconfigPath := args[1]
-	containerRootDirPath := args[2]
-
-	ldconfig, err := ldconfig.New(
-		hostLdconfigPath,
-		containerRootDirPath,
-	)
+	ldconfig, err := ldconfig.NewFromArgs(args...)
 	if err != nil {
 		return fmt.Errorf("failed to construct ldconfig runner: %w", err)
 	}
 
-	return ldconfig.UpdateLDCache(args[3:]...)
+	return ldconfig.UpdateLDCache()
 }

go.mod

@@ -8,11 +8,13 @@ require (
 	github.com/NVIDIA/go-nvlib v0.8.1
 	github.com/NVIDIA/go-nvml v0.13.0-1
 	github.com/cyphar/filepath-securejoin v0.5.0
+	github.com/moby/sys/mountinfo v0.7.2
 	github.com/moby/sys/reexec v0.1.0
 	github.com/moby/sys/symlink v0.3.0
 	github.com/opencontainers/runc v1.3.2
 	github.com/opencontainers/runtime-spec v1.2.1
 	github.com/pelletier/go-toml v1.9.5
+	github.com/prometheus/procfs v0.17.0
 	github.com/sirupsen/logrus v1.9.3
 	github.com/stretchr/testify v1.11.1
 	github.com/urfave/cli-altsrc/v3 v3.1.0
@@ -26,7 +28,6 @@ require (
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
-	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/kr/pretty v0.3.1 // indirect

go.sum

@@ -13,8 +13,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
 github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
+github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -31,6 +31,8 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/mndrix/tap-go v0.0.0-20171203230836-629fa407e90b/go.mod h1:pzzDgJWZ34fGzaAZGFW22KVZDfyrYW+QABMrWnJBnSs=
+github.com/moby/sys/mountinfo v0.7.2 h1:1shs6aH5s4o5H2zQLn796ADW1wMrIwHsyJ2v9KouLrg=
+github.com/moby/sys/mountinfo v0.7.2/go.mod h1:1YOa8w8Ih7uW0wALDUgT1dTTSBrZ+HiBLGws92L2RU4=
 github.com/moby/sys/reexec v0.1.0 h1:RrBi8e0EBTLEgfruBOFcxtElzRGTEUkeIFaVXgU7wok=
 github.com/moby/sys/reexec v0.1.0/go.mod h1:EqjBg8F3X7iZe5pU6nRZnYCMUTXoxsjiIfHup5wYIN8=
 github.com/moby/sys/symlink v0.3.0 h1:GZX89mEZ9u53f97npBy4Rc3vJKj7JBDj/PN2I22GrNU=
@@ -51,6 +53,8 @@ github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCko
 github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/prometheus/procfs v0.17.0 h1:FuLQ+05u4ZI+SS/w9+BWEM2TXiHKsUQ9TADiRH7DuK0=
+github.com/prometheus/procfs v0.17.0/go.mod h1:oPQLaDAMRbA+u8H5Pbfq+dl3VDAvHxMUOVhe0wYB2zw=
 github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
 github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
 github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=

internal/ldconfig/ldconfig.go

@@ -18,12 +18,15 @@
 package ldconfig
 
 import (
+	"flag"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
 
+	"github.com/prometheus/procfs"
+
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 )
 
@@ -39,37 +42,72 @@ const (
 type Ldconfig struct {
 	ldconfigPath string
 	inRoot       string
+	noPivotRoot  bool
+	directories  []string
 }
 
 // NewRunner creates an exec.Cmd that can be used to run ldconfig.
 func NewRunner(id string, ldconfigPath string, containerRoot string, additionalargs ...string) (*exec.Cmd, error) {
 	args := []string{
 		id,
-		strings.TrimPrefix(config.NormalizeLDConfigPath("@"+ldconfigPath), "@"),
-		containerRoot,
+		"--ldconfig-path", strings.TrimPrefix(config.NormalizeLDConfigPath("@"+ldconfigPath), "@"),
+		"--container-root", containerRoot,
+	}
+
+	if noPivotRoot() {
+		args = append(args, "--no-pivot")
 	}
+
 	args = append(args, additionalargs...)
 
 	return createReexecCommand(args)
 }
 
-// New creates an Ldconfig struct that is used to perform operations on the
-// ldcache and libraries in a particular root (e.g. a container).
-func New(ldconfigPath string, inRoot string) (*Ldconfig, error) {
-	l := &Ldconfig{
-		ldconfigPath: ldconfigPath,
-		inRoot:       inRoot,
+// NewFromArgs creates an Ldconfig struct from the args passed to the Cmd
+// above.
+// This struct is used to perform operations on the ldcache and libraries in a
+// particular root (e.g. a container).
+//
+// args[0] is the reexec initializer function name
+// The following flags are required:
+//
+//	--ldconfig-path=LDCONFIG_PATH	the path to ldconfig on the host
+//	--container-root=CONTAINER_ROOT	the path in which ldconfig must be run
+//
+// The following optional flags are supported:
+//
+//	--no-pivot	pivot_root should not be used to provide process isolation.
+//
+// The remaining args are folders where soname symlinks need to be created.
+func NewFromArgs(args ...string) (*Ldconfig, error) {
+	if len(args) < 1 {
+		return nil, fmt.Errorf("incorrect arguments: %v", args)
+	}
+	fs := flag.NewFlagSet(args[1], flag.ExitOnError)
+	ldconfigPath := fs.String("ldconfig-path", "", "the path to ldconfig on the host")
+	containerRoot := fs.String("container-root", "", "the path in which ldconfig must be run")
+	noPivot := fs.Bool("no-pivot", false, "don't use pivot_root to perform isolation")
+	if err := fs.Parse(args[1:]); err != nil {
+		return nil, err
 	}
-	if ldconfigPath == "" {
+
+	if *ldconfigPath == "" {
 		return nil, fmt.Errorf("an ldconfig path must be specified")
 	}
-	if inRoot == "" || inRoot == "/" {
+	if *containerRoot == "" || *containerRoot == "/" {
 		return nil, fmt.Errorf("ldconfig must be run in the non-system root")
 	}
+
+	l := &Ldconfig{
+		ldconfigPath: *ldconfigPath,
+		inRoot:       *containerRoot,
+		noPivotRoot:  *noPivot,
+		directories:  fs.Args(),
+	}
 	return l, nil
 }
 
-func (l *Ldconfig) UpdateLDCache(directories ...string) error {
+func (l *Ldconfig) UpdateLDCache() error {
 	ldconfigPath, err := l.prepareRoot()
 	if err != nil {
 		return err
@@ -83,7 +121,7 @@ func (l *Ldconfig) UpdateLDCache(directories ...string) error {
 		"-C", "/etc/ld.so.cache",
 	}
 
-	if err := createLdsoconfdFile(ldsoconfdFilenamePattern, directories...); err != nil {
+	if err := createLdsoconfdFile(ldsoconfdFilenamePattern, l.directories...); err != nil {
 		return fmt.Errorf("failed to update ld.so.conf.d: %w", err)
 	}
 
@@ -104,9 +142,18 @@ func (l *Ldconfig) prepareRoot() (string, error) {
 		return "", fmt.Errorf("error mounting host ldconfig: %w", err)
 	}
 
+	// We select the function to pivot the root based on whether pivot_root is
+	// supported.
+	// See https://github.com/opencontainers/runc/blob/c3d127f6e8d9f6c06d78b8329cafa8dd39f6236e/libcontainer/rootfs_linux.go#L207-L216
+	var pivotRootFn func(string) error
+	if l.noPivotRoot {
+		pivotRootFn = msMoveRoot
+	} else {
+		pivotRootFn = pivotRoot
+	}
 	// We pivot to the container root for the new process, this further limits
 	// access to the host.
-	if err := pivotRoot(l.inRoot); err != nil {
+	if err := pivotRootFn(l.inRoot); err != nil {
 		return "", fmt.Errorf("error running pivot_root: %w", err)
 	}
 
@@ -153,3 +200,39 @@ func createLdsoconfdFile(pattern string, dirs ...string) error {
 
 	return nil
 }
+
+// noPivotRoot checks whether the current root filesystem supports a pivot_root.
+// See https://github.com/opencontainers/runc/blob/main/libcontainer/SPEC.md#filesystem
+// for a discussion on when this is not the case.
+// If we fail to detect whether pivot-root is supported, we assume that it is supported.
+// The logic to check for support is adapted from kata-containers:
+//
+//	https://github.com/kata-containers/kata-containers/blob/e7b9eddcede4bbe2edeb9c3af7b2358dc65da76f/src/agent/src/sandbox.rs#L150
+//
+// and checks whether "/" is mounted as a rootfs.
+func noPivotRoot() bool {
+	rootFsType, err := getRootfsType("/")
+	if err != nil {
+		return false
+	}
+	return rootFsType == "rootfs"
+}
+
+func getRootfsType(path string) (string, error) {
+	procSelf, err := procfs.Self()
+	if err != nil {
+		return "", err
+	}
+
+	mountStats, err := procSelf.MountStats()
+	if err != nil {
+		return "", err
+	}
+
+	for _, mountStat := range mountStats {
+		if mountStat.Mount == path {
+			return mountStat.Type, nil
+		}
+	}
+	return "", fmt.Errorf("mount stats for %q not found", path)
+}

internal/ldconfig/ldconfig_linux.go

@@ -26,9 +26,11 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"syscall"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/moby/sys/mountinfo"
 	"github.com/moby/sys/reexec"
 
 	"github.com/opencontainers/runc/libcontainer/utils"
@@ -99,6 +101,86 @@ func pivotRoot(rootfs string) error {
 	return nil
 }
 
+// msMoveRoot is used
+// filesystem, and everything else is cleaned up.
+// This is adapted from the implementation here:
+//
+//	https://github.com/opencontainers/runc/blob/e89a29929c775025419ab0d218a43588b4c12b9a/libcontainer/rootfs_linux.go#L1056-L1113
+//
+// With the `mount` and `unmount` calls changed to direct unix.Mount and unix.Unmount calls.
+func msMoveRoot(rootfs string) error {
+	// Before we move the root and chroot we have to mask all "full" sysfs and
+	// procfs mounts which exist on the host. This is because while the kernel
+	// has protections against mounting procfs if it has masks, when using
+	// chroot(2) the *host* procfs mount is still reachable in the mount
+	// namespace and the kernel permits procfs mounts inside --no-pivot
+	// containers.
+	//
+	// Users shouldn't be using --no-pivot except in exceptional circumstances,
+	// but to avoid such a trivial security flaw we apply a best-effort
+	// protection here. The kernel only allows a mount of a pseudo-filesystem
+	// like procfs or sysfs if there is a *full* mount (the root of the
+	// filesystem is mounted) without any other locked mount points covering a
+	// subtree of the mount.
+	//
+	// So we try to unmount (or mount tmpfs on top of) any mountpoint which is
+	// a full mount of either sysfs or procfs (since those are the most
+	// concerning filesystems to us).
+	mountinfos, err := mountinfo.GetMounts(func(info *mountinfo.Info) (skip, stop bool) {
+		// Collect every sysfs and procfs filesystem, except for those which
+		// are non-full mounts or are inside the rootfs of the container.
+		if info.Root != "/" ||
+			(info.FSType != "proc" && info.FSType != "sysfs") ||
+			strings.HasPrefix(info.Mountpoint, rootfs) {
+			skip = true
+		}
+		return
+	})
+	if err != nil {
+		return err
+	}
+	for _, info := range mountinfos {
+		p := info.Mountpoint
+		// Be sure umount events are not propagated to the host.
+		if err := unix.Mount("", p, "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil {
+			if errors.Is(err, unix.ENOENT) {
+				// If the mountpoint doesn't exist that means that we've
+				// already blasted away some parent directory of the mountpoint
+				// and so we don't care about this error.
+				continue
+			}
+			return err
+		}
+		if err := unix.Unmount(p, unix.MNT_DETACH); err != nil {
+			if !errors.Is(err, unix.EINVAL) && !errors.Is(err, unix.EPERM) {
+				return err
+			} else {
+				// If we have not privileges for umounting (e.g. rootless), then
+				// cover the path.
+				if err := unix.Mount("tmpfs", p, "tmpfs", 0, ""); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	// Move the rootfs on top of "/" in our mount namespace.
+	if err := unix.Mount(rootfs, "/", "", unix.MS_MOVE, ""); err != nil {
+		return err
+	}
+	return chroot()
+}
+
+func chroot() error {
+	if err := unix.Chroot("."); err != nil {
+		return &os.PathError{Op: "chroot", Path: ".", Err: err}
+	}
+	if err := unix.Chdir("/"); err != nil {
+		return &os.PathError{Op: "chdir", Path: "/", Err: err}
+	}
+	return nil
+}
+
 // mountLdConfig mounts the host ldconfig to the mount namespace of the hook.
 // We use WithProcfd to perform the mount operations to ensure that the changes
 // are persisted across the pivot root.

internal/ldconfig/ldconfig_other.go

@@ -28,6 +28,10 @@ func pivotRoot(newroot string) error {
 	return fmt.Errorf("not supported")
 }
 
+func msMoveRoot(rootfs string) error {
+	return fmt.Errorf("not supported")
+}
+
 func mountLdConfig(hostLdconfigPath string, containerRootDirPath string) (string, error) {
 	return "", fmt.Errorf("not supported")
 }