From e99166acd424c9531030f9858be509ce7a43ba12 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 2 Jul 2025 14:11:19 +0200 Subject: [PATCH 1/2] [no-relnote] Refactor Ldconfig construction Signed-off-by: Evan Lezar (cherry picked from commit 3fe923de88919e95baf7ccfa4c77bc1a5053b288) --- .../update-ldcache/update-ldcache.go | 18 +------ internal/ldconfig/ldconfig.go | 48 ++++++++++++++----- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go b/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go index 136d28188..f7f86f850 100644 --- a/cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go +++ b/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() } diff --git a/internal/ldconfig/ldconfig.go b/internal/ldconfig/ldconfig.go index ba707792f..0bd8be944 100644 --- a/internal/ldconfig/ldconfig.go +++ b/internal/ldconfig/ldconfig.go @@ -18,6 +18,7 @@ package ldconfig import ( + "flag" "fmt" "os" "os/exec" @@ -39,37 +40,60 @@ const ( type Ldconfig struct { ldconfigPath string inRoot string + 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, } 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 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") + 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, + 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 +107,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) } From c40eab63f943fb01b6a772e01e8b71c2a28e6b24 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 31 Oct 2025 13:55:36 +0100 Subject: [PATCH 2/2] Filter already tracked directories from ldcache update This change fixes the behavior where the order of precedence of existing folders are changed because they are added to the .conf file for ldconfig. Signed-off-by: Evan Lezar (cherry picked from commit 1bd5242b3db6401694b44f716a9a46c7ff2b313f) --- internal/ldconfig/ldconfig.go | 174 +++++++++++++++++++-- internal/ldconfig/ldconfig_test.go | 126 +++++++++++++++ tests/e2e/nvidia-container-toolkit_test.go | 27 ++++ 3 files changed, 317 insertions(+), 10 deletions(-) create mode 100644 internal/ldconfig/ldconfig_test.go diff --git a/internal/ldconfig/ldconfig.go b/internal/ldconfig/ldconfig.go index 0bd8be944..491c69f8c 100644 --- a/internal/ldconfig/ldconfig.go +++ b/internal/ldconfig/ldconfig.go @@ -18,11 +18,13 @@ package ldconfig import ( + "bufio" "flag" "fmt" "os" "os/exec" "path/filepath" + "runtime" "strings" "github.com/NVIDIA/nvidia-container-toolkit/internal/config" @@ -38,9 +40,11 @@ const ( ) type Ldconfig struct { - ldconfigPath string - inRoot string - directories []string + ldconfigPath string + inRoot string + isDebianLikeHost bool + isDebianLikeContainer bool + directories []string } // NewRunner creates an exec.Cmd that can be used to run ldconfig. @@ -50,6 +54,9 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala "--ldconfig-path", strings.TrimPrefix(config.NormalizeLDConfigPath("@"+ldconfigPath), "@"), "--container-root", containerRoot, } + if isDebian() { + args = append(args, "--is-debian-like-host") + } args = append(args, additionalargs...) return createReexecCommand(args) @@ -66,6 +73,10 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala // --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 flags are optional: +// +// --is-debian-like-host Indicates that the host system is debian-based. +// // The remaining args are folders where soname symlinks need to be created. func NewFromArgs(args ...string) (*Ldconfig, error) { if len(args) < 1 { @@ -74,6 +85,7 @@ func NewFromArgs(args ...string) (*Ldconfig, error) { 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") + isDebianLikeHost := fs.Bool("is-debian-like-host", false, "the hook is running from a Debian-like host") if err := fs.Parse(args[1:]); err != nil { return nil, err } @@ -86,9 +98,11 @@ func NewFromArgs(args ...string) (*Ldconfig, error) { } l := &Ldconfig{ - ldconfigPath: *ldconfigPath, - inRoot: *containerRoot, - directories: fs.Args(), + ldconfigPath: *ldconfigPath, + inRoot: *containerRoot, + isDebianLikeHost: *isDebianLikeHost, + isDebianLikeContainer: isDebian(), + directories: fs.Args(), } return l, nil } @@ -99,15 +113,21 @@ func (l *Ldconfig) UpdateLDCache() error { return err } + // Explicitly specify using /etc/ld.so.conf since the host's ldconfig may + // be configured to use a different config file by default. + const topLevelLdsoconfFilePath = "/etc/ld.so.conf" + filteredDirectories, err := l.filterDirectories(topLevelLdsoconfFilePath, l.directories...) + if err != nil { + return err + } + args := []string{ filepath.Base(ldconfigPath), - // Explicitly specify using /etc/ld.so.conf since the host's ldconfig may - // be configured to use a different config file by default. - "-f", "/etc/ld.so.conf", + "-f", topLevelLdsoconfFilePath, "-C", "/etc/ld.so.cache", } - if err := createLdsoconfdFile(ldsoconfdFilenamePattern, l.directories...); err != nil { + if err := createLdsoconfdFile(ldsoconfdFilenamePattern, filteredDirectories...); err != nil { return fmt.Errorf("failed to update ld.so.conf.d: %w", err) } @@ -137,6 +157,22 @@ func (l *Ldconfig) prepareRoot() (string, error) { return ldconfigPath, nil } +func (l *Ldconfig) filterDirectories(configFilePath string, directories ...string) ([]string, error) { + ldconfigDirs, err := l.getLdsoconfDirectories(configFilePath) + if err != nil { + return nil, err + } + + var filtered []string + for _, d := range directories { + if _, ok := ldconfigDirs[d]; ok { + continue + } + filtered = append(filtered, d) + } + return filtered, nil +} + // createLdsoconfdFile creates a file at /etc/ld.so.conf.d/. // The file is created at /etc/ld.so.conf.d/{{ .pattern }} using `CreateTemp` and // contains the specified directories on each line. @@ -177,3 +213,121 @@ func createLdsoconfdFile(pattern string, dirs ...string) error { return nil } + +// getLdsoconfDirectories returns a map of ldsoconf directories to the conf +// files that refer to the directory. +func (l *Ldconfig) getLdsoconfDirectories(configFilePath string) (map[string]struct{}, error) { + ldconfigDirs := make(map[string]struct{}) + for _, d := range l.getSystemSerachPaths() { + ldconfigDirs[d] = struct{}{} + } + + processedConfFiles := make(map[string]bool) + ldsoconfFilenames := []string{configFilePath} + for len(ldsoconfFilenames) > 0 { + ldsoconfFilename := ldsoconfFilenames[0] + ldsoconfFilenames = ldsoconfFilenames[1:] + if processedConfFiles[ldsoconfFilename] { + continue + } + processedConfFiles[ldsoconfFilename] = true + + if len(ldsoconfFilename) == 0 { + continue + } + directories, includedFilenames, err := processLdsoconfFile(ldsoconfFilename) + if err != nil { + return nil, err + } + ldsoconfFilenames = append(ldsoconfFilenames, includedFilenames...) + for _, d := range directories { + ldconfigDirs[d] = struct{}{} + } + } + return ldconfigDirs, nil +} + +func (l *Ldconfig) getSystemSerachPaths() []string { + if l.isDebianLikeContainer { + debianSystemSearchPaths() + } + return nonDebianSystemSearchPaths() +} + +// processLdsoconfFile extracts the list of directories and included configs +// from the specified file. +func processLdsoconfFile(ldsoconfFilename string) ([]string, []string, error) { + ldsoconf, err := os.Open(ldsoconfFilename) + if os.IsNotExist(err) { + return nil, nil, nil + } + if err != nil { + return nil, nil, err + } + defer ldsoconf.Close() + + var directories []string + var includedFilenames []string + scanner := bufio.NewScanner(ldsoconf) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + switch { + case strings.HasPrefix(line, "#") || len(line) == 0: + continue + case strings.HasPrefix(line, "include "): + include, err := filepath.Glob(strings.TrimPrefix(line, "include ")) + if err != nil { + // We ignore invalid includes. + // TODO: How does ldconfig handle this? + continue + } + includedFilenames = append(includedFilenames, include...) + default: + directories = append(directories, line) + } + } + return directories, includedFilenames, nil +} + +func isDebian() bool { + info, err := os.Stat("/etc/debian_version") + if err != nil { + return false + } + return !info.IsDir() +} + +// nonDebianSystemSearchPaths returns the system search paths for non-Debian +// systems. +// +// This list was taken from the output of: +// +// docker run --rm -ti redhat/ubi9 /usr/lib/ld-linux-aarch64.so.1 --help | grep -A6 "Shared library search path" +func nonDebianSystemSearchPaths() []string { + return []string{"/lib64", "/usr/lib64"} +} + +// debianSystemSearchPaths returns the system search paths for Debian-like +// systems. +// +// This list was taken from the output of: +// +// docker run --rm -ti ubuntu /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 --help | grep -A6 "Shared library search path" +func debianSystemSearchPaths() []string { + var paths []string + switch runtime.GOARCH { + case "amd64": + paths = append(paths, + "/lib/x86_64-linux-gnu", + "/usr/lib/x86_64-linux-gnu", + ) + case "arm64": + paths = append(paths, + "/lib/aarch64-linux-gnu", + "/usr/lib/aarch64-linux-gnu", + ) + } + paths = append(paths, "/lib", "/usr/lib") + + return paths +} diff --git a/internal/ldconfig/ldconfig_test.go b/internal/ldconfig/ldconfig_test.go new file mode 100644 index 000000000..a22876719 --- /dev/null +++ b/internal/ldconfig/ldconfig_test.go @@ -0,0 +1,126 @@ +/** +# 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 ldconfig + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFilterDirectories(t *testing.T) { + const topLevelConf = "TOPLEVEL.conf" + + testCases := []struct { + description string + confs map[string]string // map[filename]content, must have topLevelConf key + input []string + expected []string + }{ + { + description: "all filtered", + confs: map[string]string{ + topLevelConf: ` +# some comment +/tmp/libdir1 +/tmp/libdir2 +`, + }, + input: []string{"/tmp/libdir1", "/tmp/libdir2"}, + expected: nil, + }, + { + description: "partially filtered", + confs: map[string]string{ + topLevelConf: ` +/tmp/libdir1 +`, + }, + input: []string{"/tmp/libdir1", "/tmp/libdir2"}, + expected: []string{"/tmp/libdir2"}, + }, + { + description: "none filtered", + confs: map[string]string{ + topLevelConf: ` +# empty config +`, + }, + input: []string{"/tmp/libdir1", "/tmp/libdir2"}, + expected: []string{"/tmp/libdir1", "/tmp/libdir2"}, + }, + { + description: "filter with include and comments", + confs: map[string]string{ + topLevelConf: ` +# comment +/tmp/libdir1 +include /nonexistent/pattern* +`, + }, + input: []string{"/tmp/libdir1", "/tmp/libdir2"}, + expected: []string{"/tmp/libdir2"}, + }, + { + description: "include directive picks up more dirs to filter", + confs: map[string]string{ + topLevelConf: ` +# top-level +include INCLUDED_PATTERN* +/tmp/libdir3 +`, + "INCLUDED_PATTERN0.conf": ` +/tmp/libdir2 +# another comment +/tmp/libdir4 +`, + "INCLUDED_PATTERN1.conf": ` +/tmp/libdir1 +`, + }, + input: []string{"/tmp/libdir1", "/tmp/libdir2", "/tmp/libdir3", "/tmp/libdir4", "/tmp/libdir5"}, + expected: []string{"/tmp/libdir5"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + tmpDir := t.TempDir() + + // Prepare file contents, adjusting include globs to be absolute and unique within tmpDir + for name, content := range tc.confs { + if name == topLevelConf && len(tc.confs) > 1 { + content = strings.ReplaceAll(content, "include INCLUDED_PATTERN*", "include "+tmpDir+"/INCLUDED_PATTERN*") + } + err := os.WriteFile(tmpDir+"/"+name, []byte(content), 0600) + require.NoError(t, err) + } + + topLevelConfPath := tmpDir + "/" + topLevelConf + l := &Ldconfig{ + isDebianLikeContainer: true, + } + filtered, err := l.filterDirectories(topLevelConfPath, tc.input...) + + require.NoError(t, err) + require.Equal(t, tc.expected, filtered) + }) + } +} diff --git a/tests/e2e/nvidia-container-toolkit_test.go b/tests/e2e/nvidia-container-toolkit_test.go index 80ff3663c..69086301b 100644 --- a/tests/e2e/nvidia-container-toolkit_test.go +++ b/tests/e2e/nvidia-container-toolkit_test.go @@ -543,4 +543,31 @@ EOF`) Expect(err).ToNot(HaveOccurred()) }) }) + + When("running a container an ubuntu container with specific ld.so.conf ordering", Ordered, func() { + var ( + expectedOutput string + ) + BeforeAll(func(ctx context.Context) { + _, _, err := runner.Run(`docker build -t libordering \ + - < /etc/ld.so.conf.d/00-xxx.conf +RUN ldconfig +EOF`) + Expect(err).ToNot(HaveOccurred()) + + expectedOutput, _, err = runner.Run(`docker run --rm --runtime=runc libordering bash -c "ldconfig -p | grep libc.so."`) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should not change the ordering of libraries", func(ctx context.Context) { + output, _, err := runner.Run(`docker run --rm --runtime=nvidia libordering bash -c "ldconfig -p | grep libc.so."`) + Expect(err).ToNot(HaveOccurred()) + Expect(output).To(Equal(expectedOutput)) + }) + }) })