From 4a4e7f82260c8136963f7566cc9670ca786e4f9b Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 3 Nov 2025 15:27:57 +0100 Subject: [PATCH] Switch to go 1.25 os.Root Signed-off-by: Evan Lezar --- .../disable-device-node-modification.go | 8 ++- .../params_linux.go | 43 +++------------ .../params_other.go | 7 ++- go.mod | 8 ++- internal/ldconfig/ldconfig.go | 12 +++-- internal/ldconfig/ldconfig_linux.go | 52 ++++--------------- internal/ldconfig/ldconfig_other.go | 5 +- 7 files changed, 45 insertions(+), 90 deletions(-) diff --git a/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification.go b/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification.go index 0244e4801..2a4ce5369 100644 --- a/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification.go +++ b/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) { diff --git a/cmd/nvidia-cdi-hook/disable-device-node-modification/params_linux.go b/cmd/nvidia-cdi-hook/disable-device-node-modification/params_linux.go index c438545f1..15ddccd8b 100644 --- a/cmd/nvidia-cdi-hook/disable-device-node-modification/params_linux.go +++ b/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 -} diff --git a/cmd/nvidia-cdi-hook/disable-device-node-modification/params_other.go b/cmd/nvidia-cdi-hook/disable-device-node-modification/params_other.go index 8e2829338..d6cb3e206 100644 --- a/cmd/nvidia-cdi-hook/disable-device-node-modification/params_other.go +++ b/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") } diff --git a/go.mod b/go.mod index f659550a8..809367153 100644 --- a/go.mod +++ b/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 diff --git a/internal/ldconfig/ldconfig.go b/internal/ldconfig/ldconfig.go index 491c69f8c..4c9939f1e 100644 --- a/internal/ldconfig/ldconfig.go +++ b/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) } diff --git a/internal/ldconfig/ldconfig_linux.go b/internal/ldconfig/ldconfig_linux.go index 3f5df22bf..c09926e3e 100644 --- a/internal/ldconfig/ldconfig_linux.go +++ b/internal/ldconfig/ldconfig_linux.go @@ -20,7 +20,6 @@ package ldconfig import ( - "errors" "fmt" "os" "os/exec" @@ -28,7 +27,7 @@ import ( "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, "") } diff --git a/internal/ldconfig/ldconfig_other.go b/internal/ldconfig/ldconfig_other.go index c5d452a20..5f35d4630 100644 --- a/internal/ldconfig/ldconfig_other.go +++ b/internal/ldconfig/ldconfig_other.go @@ -21,6 +21,7 @@ package ldconfig import ( "fmt" + "os" "os/exec" ) @@ -28,11 +29,11 @@ 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") }