Merge pull request #1410 from elezar/use-os-root

Switch to go 1.25 os.Root

Author: elezar

cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification.go

@@ -90,7 +90,13 @@ func run(_ context.Context, _ *cli.Command, cfg *options) error {
 		return fmt.Errorf("failed to determined container root: %w", err)
 	}
 
-	return createParamsFileInContainer(containerRootDirPath, modifiedParamsFileContents)
+	containerRoot, err := os.OpenRoot(containerRootDirPath)
+	if err != nil {
+		return fmt.Errorf("failed to open root: %w", err)
+	}
+	defer containerRoot.Close()
+
+	return createParamsFileInContainer(containerRoot, modifiedParamsFileContents)
 }
 
 func getModifiedNVIDIAParamsContents() ([]byte, error) {

cmd/nvidia-cdi-hook/disable-device-node-modification/params_linux.go

@@ -20,32 +20,32 @@
 package disabledevicenodemodification
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 
-	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/google/uuid"
 	"github.com/opencontainers/runc/libcontainer/utils"
 	"golang.org/x/sys/unix"
 )
 
-func createParamsFileInContainer(containerRootDirPath string, contents []byte) error {
-	hookScratchDirPath := "/var/run/nvidia-ctk-hook"
-	if err := utils.MkdirAllInRoot(containerRootDirPath, hookScratchDirPath, 0755); err != nil {
+func createParamsFileInContainer(containerRoot *os.Root, contents []byte) error {
+	containerRootDirPath := containerRoot.Name()
+
+	hookScratchDirPath := "/run/nvidia-ctk-hook" + uuid.NewString()
+	if err := containerRoot.MkdirAll(hookScratchDirPath[1:], 0755); err != nil {
 		return fmt.Errorf("error creating hook scratch folder: %w", err)
 	}
 
 	err := utils.WithProcfd(containerRootDirPath, hookScratchDirPath, func(hookScratchDirFdPath string) error {
 		return createTmpFs(hookScratchDirFdPath, len(contents))
-
 	})
 	if err != nil {
 		return fmt.Errorf("failed to create tmpfs mount for params file: %w", err)
 	}
 
 	modifiedParamsFilePath := filepath.Join(hookScratchDirPath, "nvct-params")
-	if _, err := createFileInRoot(containerRootDirPath, modifiedParamsFilePath, 0444); err != nil {
+	if _, err := containerRoot.OpenFile(modifiedParamsFilePath[1:], os.O_CREATE|os.O_RDONLY|os.O_TRUNC, 0444); err != nil {
 		return fmt.Errorf("error creating modified params file: %w", err)
 	}
 
@@ -79,32 +79,3 @@ func createParamsFileInContainer(containerRootDirPath string, contents []byte) e
 func createTmpFs(target string, size int) error {
 	return unix.Mount("tmpfs", target, "tmpfs", 0, fmt.Sprintf("size=%d", size))
 }
-
-// TODO(ArangoGutierrez): This function also exists in internal/ldconfig we should move this to a separate package.
-func createFileInRoot(containerRootDirPath string, destinationPath string, mode os.FileMode) (string, error) {
-	dest, err := securejoin.SecureJoin(containerRootDirPath, destinationPath)
-	if err != nil {
-		return "", err
-	}
-	// Make the parent directory.
-	destDir, destBase := filepath.Split(dest)
-	destDirFd, err := utils.MkdirAllInRootOpen(containerRootDirPath, destDir, 0755)
-	if err != nil {
-		return "", fmt.Errorf("error creating parent dir: %w", err)
-	}
-	defer destDirFd.Close()
-	// Make the target file. We want to avoid opening any file that is
-	// already there because it could be a "bad" file like an invalid
-	// device or hung tty that might cause a DoS, so we use mknodat.
-	// destBase does not contain any "/" components, and mknodat does
-	// not follow trailing symlinks, so we can safely just call mknodat
-	// here.
-	if err := unix.Mknodat(int(destDirFd.Fd()), destBase, unix.S_IFREG|uint32(mode), 0); err != nil {
-		// If we get EEXIST, there was already an inode there and
-		// we can consider that a success.
-		if !errors.Is(err, unix.EEXIST) {
-			return "", fmt.Errorf("error creating empty file: %w", err)
-		}
-	}
-	return dest, nil
-}

cmd/nvidia-cdi-hook/disable-device-node-modification/params_other.go

@@ -19,8 +19,11 @@
 
 package disabledevicenodemodification
 
-import "fmt"
+import (
+	"fmt"
+	"os"
+)
 
-func createParamsFileInContainer(containerRootDirPath string, contents []byte) error {
+func createParamsFileInContainer(containerRootDirPath *os.Root, contents []byte) error {
 	return fmt.Errorf("not supported")
 }

go.mod

@@ -1,13 +1,11 @@
 module github.com/NVIDIA/nvidia-container-toolkit
 
-go 1.24.0
-
-toolchain go1.24.4
+go 1.25.0
 
 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/google/uuid v1.6.0
 	github.com/moby/sys/reexec v0.1.0
 	github.com/moby/sys/symlink v0.3.0
 	github.com/opencontainers/runc v1.3.2
@@ -24,10 +22,10 @@ require (
 )
 
 require (
+	github.com/cyphar/filepath-securejoin v0.5.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/google/uuid v1.6.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/kr/pretty v0.3.1 // indirect
 	github.com/opencontainers/runtime-tools v0.9.1-0.20221107090550-2e043c6bd626 // indirect

internal/ldconfig/ldconfig.go

@@ -135,22 +135,28 @@ func (l *Ldconfig) UpdateLDCache() error {
 }
 
 func (l *Ldconfig) prepareRoot() (string, error) {
+	root, err := os.OpenRoot(l.inRoot)
+	if err != nil {
+		return "", fmt.Errorf("failed to open root: %w", err)
+	}
+	defer root.Close()
+
 	// To prevent leaking the parent proc filesystem, we create a new proc mount
 	// in the specified root.
-	if err := mountProc(l.inRoot); err != nil {
+	if err := mountProc(root); err != nil {
 		return "", fmt.Errorf("error mounting /proc: %w", err)
 	}
 
 	// We mount the host ldconfig before we pivot root since host paths are not
 	// visible after the pivot root operation.
-	ldconfigPath, err := mountLdConfig(l.ldconfigPath, l.inRoot)
+	ldconfigPath, err := mountLdConfig(l.ldconfigPath, root)
 	if err != nil {
 		return "", fmt.Errorf("error mounting host ldconfig: %w", err)
 	}
 
 	// 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 := pivotRoot(root.Name()); err != nil {
 		return "", fmt.Errorf("error running pivot_root: %w", err)
 	}
 

internal/ldconfig/ldconfig_linux.go

@@ -20,15 +20,14 @@
 package ldconfig
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
 	"syscall"
 
-	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/google/uuid"
 	"github.com/moby/sys/reexec"
 
 	"github.com/opencontainers/runc/libcontainer/utils"
@@ -102,27 +101,28 @@ func pivotRoot(rootfs string) error {
 // 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.
-func mountLdConfig(hostLdconfigPath string, containerRootDirPath string) (string, error) {
+func mountLdConfig(hostLdconfigPath string, containerRoot *os.Root) (string, error) {
+	containerRootDirPath := containerRoot.Name()
+
 	hostLdconfigInfo, err := os.Stat(hostLdconfigPath)
 	if err != nil {
 		return "", fmt.Errorf("error reading host ldconfig: %w", err)
 	}
 
-	hookScratchDirPath := "/var/run/nvidia-ctk-hook"
+	hookScratchDirPath := "/run/nvidia-ctk-hook/" + uuid.NewString()
 	ldconfigPath := filepath.Join(hookScratchDirPath, "ldconfig")
-	if err := utils.MkdirAllInRoot(containerRootDirPath, hookScratchDirPath, 0755); err != nil {
+	if err := containerRoot.MkdirAll(hookScratchDirPath[1:], 0755); err != nil {
 		return "", fmt.Errorf("error creating hook scratch folder: %w", err)
 	}
 
 	err = utils.WithProcfd(containerRootDirPath, hookScratchDirPath, func(hookScratchDirFdPath string) error {
 		return createTmpFs(hookScratchDirFdPath, int(hostLdconfigInfo.Size()))
-
 	})
 	if err != nil {
 		return "", fmt.Errorf("error creating tmpfs: %w", err)
 	}
 
-	if _, err := createFileInRoot(containerRootDirPath, ldconfigPath, hostLdconfigInfo.Mode()); err != nil {
+	if _, err := containerRoot.OpenFile(ldconfigPath[1:], os.O_CREATE|os.O_RDWR|os.O_TRUNC, hostLdconfigInfo.Mode()); err != nil {
 		return "", fmt.Errorf("error creating ldconfig: %w", err)
 	}
 
@@ -136,43 +136,13 @@ func mountLdConfig(hostLdconfigPath string, containerRootDirPath string) (string
 	return ldconfigPath, nil
 }
 
-func createFileInRoot(containerRootDirPath string, destinationPath string, mode os.FileMode) (string, error) {
-	dest, err := securejoin.SecureJoin(containerRootDirPath, destinationPath)
-	if err != nil {
-		return "", err
-	}
-	// Make the parent directory.
-	destDir, destBase := filepath.Split(dest)
-	destDirFd, err := utils.MkdirAllInRootOpen(containerRootDirPath, destDir, 0755)
-	if err != nil {
-		return "", fmt.Errorf("error creating parent dir: %w", err)
-	}
-	defer destDirFd.Close()
-	// Make the target file. We want to avoid opening any file that is
-	// already there because it could be a "bad" file like an invalid
-	// device or hung tty that might cause a DoS, so we use mknodat.
-	// destBase does not contain any "/" components, and mknodat does
-	// not follow trailing symlinks, so we can safely just call mknodat
-	// here.
-	if err := unix.Mknodat(int(destDirFd.Fd()), destBase, unix.S_IFREG|uint32(mode), 0); err != nil {
-		// If we get EEXIST, there was already an inode there and
-		// we can consider that a success.
-		if !errors.Is(err, unix.EEXIST) {
-			return "", fmt.Errorf("error creating empty file: %w", err)
-		}
-	}
-	return dest, nil
-}
-
 // mountProc mounts a clean proc filesystem in the new root.
-func mountProc(newroot string) error {
-	target, err := securejoin.SecureJoin(newroot, "proc")
-	if err != nil {
-		return err
-	}
-	if err := utils.MkdirAllInRoot(newroot, target, 0755); err != nil {
+func mountProc(newroot *os.Root) error {
+	if err := newroot.MkdirAll("proc", 0755); err != nil {
 		return err
 	}
+
+	target := filepath.Join(newroot.Name(), "proc")
 	return unix.Mount("proc", target, "proc", 0, "")
 }
 

internal/ldconfig/ldconfig_other.go

@@ -21,18 +21,19 @@ package ldconfig
 
 import (
 	"fmt"
+	"os"
 	"os/exec"
 )
 
 func pivotRoot(newroot string) error {
 	return fmt.Errorf("not supported")
 }
 
-func mountLdConfig(hostLdconfigPath string, containerRootDirPath string) (string, error) {
+func mountLdConfig(hostLdconfigPath string, containerRoot *os.Root) (string, error) {
 	return "", fmt.Errorf("not supported")
 }
 
-func mountProc(newroot string) error {
+func mountProc(newroot *os.Root) error {
 	return fmt.Errorf("not supported")
 }