Merge pull request #1174 from elezar/no-pivot-root

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

Author: elezar

go.mod

@@ -6,11 +6,13 @@ require (
 	github.com/NVIDIA/go-nvlib v0.8.1
 	github.com/NVIDIA/go-nvml v0.13.0-1
 	github.com/google/uuid v1.6.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.3
 	github.com/opencontainers/runtime-spec v1.3.0
 	github.com/pelletier/go-toml v1.9.5
+	github.com/prometheus/procfs v0.19.2
 	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 (
 	github.com/cyphar/filepath-securejoin v0.6.0 // indirect
 	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/hashicorp/errwrap v1.1.0 // indirect
 	github.com/kr/pretty v0.3.1 // indirect
 	github.com/moby/sys/capability v0.4.0 // indirect

go.sum

@@ -15,8 +15,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.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
@@ -32,6 +32,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/moby/sys/capability v0.4.0 h1:4D4mI6KlNtWMCM1Z/K0i7RV1FkX+DBDHKVJpCndZoHk=
 github.com/moby/sys/capability v0.4.0/go.mod h1:4g9IK291rVkms3LKCDOoYlnV8xKwoDTpIrNEE35Wq0I=
+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.19.2 h1:zUMhqEW66Ex7OXIiDkll3tl9a1ZdilUOd/F6ZXw4Vws=
+github.com/prometheus/procfs v0.19.2/go.mod h1:M0aotyiemPhBCM0z5w87kL22CxfcH05ZpYlu+b4J7mw=
 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

@@ -27,6 +27,8 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/prometheus/procfs"
+
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 )
 
@@ -59,6 +61,7 @@ type Ldconfig struct {
 	inRoot                string
 	isDebianLikeHost      bool
 	isDebianLikeContainer bool
+	noPivotRoot           bool
 	directories           []string
 }
 
@@ -72,6 +75,11 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala
 	if isDebianLike() {
 		args = append(args, "--is-debian-like-host")
 	}
+
+	if noPivotRoot() {
+		args = append(args, "--no-pivot")
+	}
+
 	args = append(args, additionalargs...)
 
 	return createReexecCommand(args)
@@ -94,6 +102,7 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala
 //	--is-debian-like-host	Indicates that the host system is debian-like (e.g. Debian, Ubuntu)
 //	                     	as opposed to non-Debian-like (e.g. RHEL, Fedora)
 //	                     	See https://github.com/NVIDIA/nvidia-container-toolkit/pull/1444
+//	--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) {
@@ -107,6 +116,7 @@ func NewFromArgs(args ...string) (*Ldconfig, error) {
 This allows us to handle the case where there are  differences in behavior
 between the ldconfig from the host (as executed from an update-ldcache hook) and
 ldconfig in the container. Such differences include system search paths.`)
+	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
 	}
@@ -122,6 +132,7 @@ ldconfig in the container. Such differences include system search paths.`)
 		ldconfigPath:     *ldconfigPath,
 		inRoot:           *containerRoot,
 		isDebianLikeHost: *isDebianLikeHost,
+		noPivotRoot:      *noPivot,
 		directories:      fs.Args(),
 	}
 	return l, nil
@@ -194,7 +205,7 @@ func (l *Ldconfig) prepareRoot() (string, error) {
 
 	// We pivot to the container root for the new process, this further limits
 	// access to the host.
-	if err := pivotRoot(root.Name()); err != nil {
+	if err := l.pivotRoot(root); err != nil {
 		return "", fmt.Errorf("error running pivot_root: %w", err)
 	}
 
@@ -456,3 +467,50 @@ func debianSystemSearchPaths() []string {
 	paths = append(paths, "/lib", "/usr/lib")
 	return paths
 }
+
+func (l *Ldconfig) pivotRoot(root *os.Root) error {
+	rootDir := root.Name()
+	// 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
+	if l.noPivotRoot {
+		return msMoveRoot(rootDir)
+	}
+	return pivotRoot(rootDir)
+}
+
+// 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

@@ -20,14 +20,17 @@
 package ldconfig
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"syscall"
 
 	"github.com/google/uuid"
+	"github.com/moby/sys/mountinfo"
 	"github.com/moby/sys/reexec"
 
 	"github.com/opencontainers/runc/libcontainer/utils"
@@ -98,6 +101,86 @@ func pivotRoot(rootfs string) error {
 	return nil
 }
 
+// msMoveRoot is used in cases where pivot root is not supported.
+// This includes initramfs filesystems where the root is read-only.
+// This is adapted from the implementation here:
+//
+//	https://github.com/opencontainers/runc/blob/e89a29929c775025419ab0d218a43588b4c12b9a/libcontainer/rootfs_linux.go#L1115
+//
+// 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

@@ -29,6 +29,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, containerRoot *os.Root) (string, error) {
 	return "", fmt.Errorf("not supported")
 }