diff --git a/cmd/nvidia-cdi-hook/commands/commands.go b/cmd/nvidia-cdi-hook/commands/commands.go index 3f80ba9be..66bb43956 100644 --- a/cmd/nvidia-cdi-hook/commands/commands.go +++ b/cmd/nvidia-cdi-hook/commands/commands.go @@ -22,6 +22,7 @@ import ( "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/chmod" symlinks "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/create-symlinks" "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/cudacompat" + disabledevicenodemodification "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/disable-device-node-modification" ldcache "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/update-ldcache" "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" ) @@ -34,5 +35,6 @@ func New(logger logger.Interface) []*cli.Command { symlinks.NewCommand(logger), chmod.NewCommand(logger), cudacompat.NewCommand(logger), + disabledevicenodemodification.NewCommand(logger), } } 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 new file mode 100644 index 000000000..ec3ef7c59 --- /dev/null +++ b/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification.go @@ -0,0 +1,198 @@ +/** +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package disabledevicenodemodification + +import ( + "bufio" + "bytes" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/urfave/cli/v2" + + "github.com/NVIDIA/nvidia-container-toolkit/internal/logger" + "github.com/NVIDIA/nvidia-container-toolkit/internal/oci" +) + +const ( + nvidiaDriverParamsPath = "/proc/driver/nvidia/params" +) + +type command struct { + logger logger.Interface +} + +type options struct { + containerSpec string +} + +// NewCommand constructs an disable-device-node-modification subcommand with the specified logger +func NewCommand(logger logger.Interface) *cli.Command { + c := command{ + logger: logger, + } + return c.build() +} + +func (m command) build() *cli.Command { + cfg := options{} + + c := cli.Command{ + Name: "disable-device-node-modification", + Usage: "Ensure that the /proc/driver/nvidia/params file present in the container does not allow device node modifications.", + Before: func(c *cli.Context) error { + return m.validateFlags(c, &cfg) + }, + Action: func(c *cli.Context) error { + return m.run(c, &cfg) + }, + } + + c.Flags = []cli.Flag{ + &cli.StringFlag{ + Name: "container-spec", + Hidden: true, + Usage: "Specify the path to the OCI container spec. If empty or '-' the spec will be read from STDIN", + Destination: &cfg.containerSpec, + }, + } + + return &c +} + +func (m command) validateFlags(c *cli.Context, cfg *options) error { + return nil +} + +func (m command) run(c *cli.Context, cfg *options) error { + // TODO: Do we need to prefix the driver root? + hostNvidiaParamsFile, err := os.Open(nvidiaDriverParamsPath) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("failed to load params file: %w", err) + } + defer hostNvidiaParamsFile.Close() + + s, err := oci.LoadContainerState(cfg.containerSpec) + if err != nil { + return fmt.Errorf("failed to load container state: %w", err) + } + + containerRoot, err := s.GetContainerRoot() + if err != nil { + return fmt.Errorf("failed to determined container root: %w", err) + } + + return m.updateNvidiaParamsFromReader(hostNvidiaParamsFile, containerRoot) +} + +func (m command) updateNvidiaParamsFromReader(r io.Reader, containerRoot string) error { + modifiedContents, err := m.getModifiedParamsFileContentsFromReader(r) + if err != nil { + return fmt.Errorf("failed to generate modified contents: %w", err) + } + if len(modifiedContents) == 0 { + m.logger.Debugf("No modification required") + return nil + } + return createParamsFileInContainer(containerRoot, modifiedContents) +} + +// getModifiedParamsFileContentsFromReader returns the contents of a modified params file from the specified reader. +func (m command) getModifiedParamsFileContentsFromReader(r io.Reader) ([]byte, error) { + var modified bytes.Buffer + scanner := bufio.NewScanner(r) + + var requiresModification bool + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "ModifyDeviceFiles: ") { + if line == "ModifyDeviceFiles: 0" { + m.logger.Debugf("Device node modification is already disabled") + return nil, nil + } + if line == "ModifyDeviceFiles: 1" { + line = "ModifyDeviceFiles: 0" + requiresModification = true + } + } + if _, err := modified.WriteString(line + "\n"); err != nil { + return nil, fmt.Errorf("failed to create output buffer: %w", err) + } + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("failed to read params file: %w", err) + } + + if !requiresModification { + return nil, nil + } + + return modified.Bytes(), nil +} + +func createParamsFileInContainer(containerRoot string, contents []byte) error { + if len(contents) == 0 { + return nil + } + + tempParamsFileName, err := createFileInTempfs("nvct-params", contents, 0o444) + if err != nil { + return fmt.Errorf("failed to create temporary file: %w", err) + } + + if err := bindMountReadonly(tempParamsFileName, filepath.Join(containerRoot, nvidiaDriverParamsPath)); err != nil { + return fmt.Errorf("failed to create temporary params file mount: %w", err) + } + + return nil +} + +// createFileInTempfs creates a file with the specified name, contents, and mode in a tmpfs. +// A tmpfs is created at /tmp/nvct-emtpy-dir* with a size sufficient for the specified contents. +func createFileInTempfs(name string, contents []byte, mode os.FileMode) (string, error) { + tmpRoot, err := os.MkdirTemp("", "nvct-empty-dir*") + if err != nil { + return "", fmt.Errorf("failed to create temporary folder: %w", err) + } + if err := createTmpFs(tmpRoot, len(contents)); err != nil { + return "", fmt.Errorf("failed to create tmpfs mount for params file: %w", err) + } + + filename := filepath.Join(tmpRoot, name) + fileInTempfs, err := os.Create(filename) + if err != nil { + return "", fmt.Errorf("failed to create temporary params file: %w", err) + } + defer fileInTempfs.Close() + + if _, err := fileInTempfs.Write(contents); err != nil { + return "", fmt.Errorf("failed to write temporary params file: %w", err) + } + + if err := fileInTempfs.Chmod(mode); err != nil { + return "", fmt.Errorf("failed to set permissions on temporary params file: %w", err) + } + return filename, nil +} diff --git a/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification_test.go b/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification_test.go new file mode 100644 index 000000000..24c5583f5 --- /dev/null +++ b/cmd/nvidia-cdi-hook/disable-device-node-modification/disable-device-node-modification_test.go @@ -0,0 +1,96 @@ +/** +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package disabledevicenodemodification + +import ( + "bytes" + "testing" + + testlog "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/require" +) + +func TestGetModifiedParamsFileContentsFromReader(t *testing.T) { + logger, _ := testlog.NewNullLogger() + testCases := map[string]struct { + contents []byte + expectedError error + expectedContents []byte + }{ + "no contents": { + contents: nil, + expectedError: nil, + expectedContents: nil, + }, + "other contents are ignored": { + contents: []byte(`# Some other content + that we don't care about + `), + expectedError: nil, + expectedContents: nil, + }, + "already zero requires no modification": { + contents: []byte("ModifyDeviceFiles: 0"), + expectedError: nil, + expectedContents: nil, + }, + "leading spaces require no modification": { + contents: []byte(" ModifyDeviceFiles: 1"), + }, + "Trailing spaces require no modification": { + contents: []byte("ModifyDeviceFiles: 1 "), + }, + "Not 1 require no modification": { + contents: []byte("ModifyDeviceFiles: 11"), + }, + "single line requires modification": { + contents: []byte("ModifyDeviceFiles: 1"), + expectedError: nil, + expectedContents: []byte("ModifyDeviceFiles: 0\n"), + }, + "single line with trailing newline requires modification": { + contents: []byte("ModifyDeviceFiles: 1\n"), + expectedError: nil, + expectedContents: []byte("ModifyDeviceFiles: 0\n"), + }, + "other content is maintained": { + contents: []byte(`ModifyDeviceFiles: 1 + other content + that + is maintained`), + expectedError: nil, + expectedContents: []byte(`ModifyDeviceFiles: 0 + other content + that + is maintained +`), + }, + } + + for description, tc := range testCases { + t.Run(description, func(t *testing.T) { + c := command{ + logger: logger, + } + contents, err := c.getModifiedParamsFileContentsFromReader(bytes.NewReader(tc.contents)) + require.EqualValues(t, tc.expectedError, err) + require.EqualValues(t, string(tc.expectedContents), string(contents)) + }) + } + +} diff --git a/cmd/nvidia-cdi-hook/disable-device-node-modification/mount_linux.go b/cmd/nvidia-cdi-hook/disable-device-node-modification/mount_linux.go new file mode 100644 index 000000000..92d678134 --- /dev/null +++ b/cmd/nvidia-cdi-hook/disable-device-node-modification/mount_linux.go @@ -0,0 +1,35 @@ +//go:build linux +// +build linux + +/** +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package disabledevicenodemodification + +import ( + "fmt" + + "golang.org/x/sys/unix" +) + +func createTmpFs(target string, size int) error { + return unix.Mount("tmpfs", target, "tmpfs", 0, fmt.Sprintf("size=%d", size)) +} + +func bindMountReadonly(source string, target string) error { + return unix.Mount(source, target, "", unix.MS_BIND|unix.MS_RDONLY|unix.MS_NOSYMFOLLOW, "") +} diff --git a/cmd/nvidia-cdi-hook/disable-device-node-modification/mount_other.go b/cmd/nvidia-cdi-hook/disable-device-node-modification/mount_other.go new file mode 100644 index 000000000..cb92b0488 --- /dev/null +++ b/cmd/nvidia-cdi-hook/disable-device-node-modification/mount_other.go @@ -0,0 +1,33 @@ +//go:build !linux +// +build !linux + +/** +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package disabledevicenodemodification + +import ( + "fmt" +) + +func createTmpFs(target string, size int) error { + return fmt.Errorf("not supported") +} + +func bindMountReadonly(source string, target string) error { + return fmt.Errorf("not supported") +} diff --git a/cmd/nvidia-ctk/cdi/generate/generate_test.go b/cmd/nvidia-ctk/cdi/generate/generate_test.go index 9e9a6a4e4..02b9ffaa3 100644 --- a/cmd/nvidia-ctk/cdi/generate/generate_test.go +++ b/cmd/nvidia-ctk/cdi/generate/generate_test.go @@ -104,6 +104,11 @@ containerEdits: - update-ldcache - --folder - /lib/x86_64-linux-gnu + - hookName: createContainer + path: /usr/bin/nvidia-cdi-hook + args: + - nvidia-cdi-hook + - disable-device-node-modification mounts: - hostPath: {{ .driverRoot }}/lib/x86_64-linux-gnu/libcuda.so.999.88.77 containerPath: /lib/x86_64-linux-gnu/libcuda.so.999.88.77 diff --git a/pkg/nvcdi/api.go b/pkg/nvcdi/api.go index 2988026f3..86b45e255 100644 --- a/pkg/nvcdi/api.go +++ b/pkg/nvcdi/api.go @@ -44,4 +44,7 @@ const ( // HookEnableCudaCompat refers to the hook used to enable CUDA Forward Compatibility. // This was added with v1.17.5 of the NVIDIA Container Toolkit. HookEnableCudaCompat = HookName("enable-cuda-compat") + // HookDisableDeviceNodeModification refers to the hook used to ensure that device nodes + // are not created by nvidia-smi in a container. + HookDisableDeviceNodeModification = HookName("disable-device-node-modification") ) diff --git a/pkg/nvcdi/driver-nvml.go b/pkg/nvcdi/driver-nvml.go index f49f1129b..954483df7 100644 --- a/pkg/nvcdi/driver-nvml.go +++ b/pkg/nvcdi/driver-nvml.go @@ -115,6 +115,14 @@ func (l *nvcdilib) NewDriverLibraryDiscoverer(version string) (discover.Discover updateLDCache, _ := discover.NewLDCacheUpdateHook(l.logger, libraries, l.nvidiaCDIHookPath, l.ldconfigPath) discoverers = append(discoverers, updateLDCache) + if l.HookIsSupported(HookDisableDeviceNodeModification) { + updateNvidiaParams := discover.CreateNvidiaCDIHook( + l.nvidiaCDIHookPath, + "disable-device-node-modification", + ) + discoverers = append(discoverers, updateNvidiaParams) + } + d := discover.Merge(discoverers...) return d, nil diff --git a/pkg/nvcdi/lib.go b/pkg/nvcdi/lib.go index 8e7653b44..94793a4a2 100644 --- a/pkg/nvcdi/lib.go +++ b/pkg/nvcdi/lib.go @@ -146,6 +146,9 @@ func New(opts ...Option) (Interface, error) { } // Management containers in general do not require CUDA Forward compatibility. l.disabledHooks[HookEnableCudaCompat] = true + // For Management containers we allow device node creation: + l.disabledHooks[HookDisableDeviceNodeModification] = true + lib = (*managementlib)(l) case ModeNvml: lib = (*nvmllib)(l) diff --git a/tests/e2e/nvidia-container-toolkit_test.go b/tests/e2e/nvidia-container-toolkit_test.go index 5948014b6..815aafee9 100644 --- a/tests/e2e/nvidia-container-toolkit_test.go +++ b/tests/e2e/nvidia-container-toolkit_test.go @@ -215,4 +215,23 @@ var _ = Describe("docker", Ordered, ContinueOnFailure, func() { Expect(ldconfigOut).To(ContainSubstring("/usr/lib64")) }) }) + + Describe("Disabling device node creation", Ordered, func() { + BeforeAll(func(ctx context.Context) { + _, _, err := r.Run("docker pull ubuntu") + Expect(err).ToNot(HaveOccurred()) + }) + + It("should work with nvidia-container-runtime-hook", func(ctx context.Context) { + output, _, err := r.Run("docker run --rm -i --runtime=runc --gpus=all ubuntu bash -c \"grep ModifyDeviceFiles: /proc/driver/nvidia/params\"") + Expect(err).ToNot(HaveOccurred()) + Expect(output).To(Equal("ModifyDeviceFiles: 0\n")) + }) + + It("should work with automatic CDI spec generation", func(ctx context.Context) { + output, _, err := r.Run("docker run --rm -i --runtime=nvidia -e NVIDIA_VISIBLE_DEVICES=runtime.nvidia.com/gpu=all ubuntu bash -c \"grep ModifyDeviceFiles: /proc/driver/nvidia/params\"") + Expect(err).ToNot(HaveOccurred()) + Expect(output).To(Equal("ModifyDeviceFiles: 0\n")) + }) + }) })