Append container system paths to ld.so.conf

internal/ldconfig/ldconfig.go

@@ -67,7 +67,8 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala
 // 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
+// args[0] is the reexec initializer function name and is required.
+//
 // The following flags are required:
 //
 //	--ldconfig-path=LDCONFIG_PATH	the path to ldconfig on the host
@@ -76,16 +77,20 @@ func NewRunner(id string, ldconfigPath string, containerRoot string, additionala
 // The following flags are optional:
 //
 //	--is-debian-like-host	Indicates that the host system is debian-based.
+//							See https://github.com/NVIDIA/nvidia-container-toolkit/pull/1444
 //
 // 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)
+	fs := flag.NewFlagSet("ldconfig-options", 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")
+	isDebianLikeHost := fs.Bool("is-debian-like-host", false, `indicates that the host system is debian-based.
+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.`)
 	if err := fs.Parse(args[1:]); err != nil {
 		return nil, err
 	}
@@ -98,11 +103,10 @@ func NewFromArgs(args ...string) (*Ldconfig, error) {
 	}
 
 	l := &Ldconfig{
-		ldconfigPath:          *ldconfigPath,
-		inRoot:                *containerRoot,
-		isDebianLikeHost:      *isDebianLikeHost,
-		isDebianLikeContainer: isDebian(),
-		directories:           fs.Args(),
+		ldconfigPath:     *ldconfigPath,
+		inRoot:           *containerRoot,
+		isDebianLikeHost: *isDebianLikeHost,
+		directories:      fs.Args(),
 	}
 	return l, nil
 }
@@ -113,6 +117,9 @@ func (l *Ldconfig) UpdateLDCache() error {
 		return err
 	}
 
+	// `prepareRoot` pivots to the container root, so can now set the container "debian-ness".
+	l.isDebianLikeContainer = isDebian()
+
 	// 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"
@@ -131,6 +138,16 @@ func (l *Ldconfig) UpdateLDCache() error {
 		return fmt.Errorf("failed to update ld.so.conf.d: %w", err)
 	}
 
+	// In most cases, the hook will be executing a host ldconfig that may be configured widely
+	// differently from what the container image expects. The common case is Debian vs non-Debian.
+	// But there are also hosts that configure ldconfig to search in a glibc prefix
+	// (e.g. /usr/lib/glibc). To avoid all these cases, append the container's expected system
+	// search paths to the top-level ld.so.conf. This will ensure they get scanned but won't
+	// materially change the scan order.
+	if err := appendSystemSearchPathsToLdsoconf(topLevelLdsoconfFilePath, l.getSystemSearchPaths()...); err != nil {
+		return fmt.Errorf("failed to append system search paths to %s: %w", topLevelLdsoconfFilePath, err)
+	}
+
 	return SafeExec(ldconfigPath, args, nil)
 }
 
@@ -169,6 +186,7 @@ func (l *Ldconfig) filterDirectories(configFilePath string, directories ...strin
 			continue
 		}
 		filtered = append(filtered, d)
+		ldconfigDirs[d] = struct{}{}
 	}
 	return filtered, nil
 }
@@ -214,11 +232,29 @@ func createLdsoconfdFile(pattern string, dirs ...string) error {
 	return nil
 }
 
+func appendSystemSearchPathsToLdsoconf(configFilePath string, dirs ...string) error {
+	if len(dirs) == 0 {
+		return nil
+	}
+	configFile, err := os.OpenFile(configFilePath, os.O_APPEND|os.O_WRONLY, 0644)
+	if err != nil {
+		return fmt.Errorf("failed to open config file: %w", err)
+	}
+	defer configFile.Close()
+	for _, dir := range dirs {
+		_, err = fmt.Fprintf(configFile, "%s\n", dir)
+		if err != nil {
+			return fmt.Errorf("failed to update config file: %w", err)
+		}
+	}
+	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() {
+	for _, d := range l.getSystemSearchPaths() {
 		ldconfigDirs[d] = struct{}{}
 	}
 
@@ -247,9 +283,9 @@ func (l *Ldconfig) getLdsoconfDirectories(configFilePath string) (map[string]str
 	return ldconfigDirs, nil
 }
 
-func (l *Ldconfig) getSystemSerachPaths() []string {
+func (l *Ldconfig) getSystemSearchPaths() []string {
 	if l.isDebianLikeContainer {
-		debianSystemSearchPaths()
+		return debianSystemSearchPaths()
 	}
 	return nonDebianSystemSearchPaths()
 }
@@ -297,22 +333,61 @@ func isDebian() bool {
 	return !info.IsDir()
 }
 
-// nonDebianSystemSearchPaths returns the system search paths for non-Debian
-// systems.
+// nonDebianSystemSearchPaths returns the system search paths for non-Debian systems.
+//
+// glibc ldconfig's calls `add_system_dir` with `SLIBDIR` and `LIBDIR` (if they are not equal). On
+// aarch64 and x86_64, `add_system_dir` is a macro that scans the provided path. If the path ends
+// with "/lib64" (or "/libx32", x86_64 only), it strips those suffixes. Then it registers the
+// resulting path. Then if the path ends with "/lib", it registers "path"+"64" (and "path"+"x32",
+// x86_64 only).
+//
+// By default, "LIBDIR" is "/usr/lib" and "SLIBDIR" is "/lib". Note that on modern distributions,
+// "/lib" is usually a symlink to "/usr/lib" and "/lib64" to "/usr/lib64". ldconfig resolves
+// symlinks and skips duplicate directory entries.
 //
-// This list was taken from the output of:
+// To get the list of system paths, you can invoke the dynamic linker with `--list-diagnostics` and
+// look for "path.system_dirs". For example
+// `docker run --rm -ti fedora:latest /lib64/ld-linux-x86-64.so.2 --list-diagnostics | grep path.system_dirs`.
 //
-//	docker run --rm -ti redhat/ubi9 /usr/lib/ld-linux-aarch64.so.1 --help | grep -A6 "Shared library search path"
+// On most distributions, including Fedora and derivatives, this yields the following
+// ldconfig system search paths.
+//
+// TODO: Add other architectures that have custom `add_system_dir` macros (e.g. riscv)
+// TODO: Replace with executing the container's dynamlic linker with `--list-diagnostics`?
 func nonDebianSystemSearchPaths() []string {
-	return []string{"/lib64", "/usr/lib64"}
+	var paths []string
+	paths = append(paths, "/lib", "/usr/lib")
+	switch runtime.GOARCH {
+	case "amd64":
+		paths = append(paths,
+			"/lib/lib64",
+			"/usr/lib64",
+			"/libx32",
+			"/usr/libx32",
+		)
+	case "arm64":
+		paths = append(paths,
+			"/lib/lib64",
+			"/usr/lib64",
+		)
+	}
+	return paths
 }
 
-// debianSystemSearchPaths returns the system search paths for Debian-like
-// systems.
+// debianSystemSearchPaths returns the system search paths for Debian-like systems.
+//
+// Debian (and derivatives) apply their multi-arch patch to glibc, which modifies ldconfig to
+// use the same set of system paths as the dynamic linker. These paths are going to include the
+// multi-arch directory _and_ by default "/lib" and "/usr/lib" for compatibility.
 //
-// This list was taken from the output of:
+// To get the list of system paths, you can invoke the dynamic linker with `--list-diagnostics` and
+// look for "path.system_dirs". For example
+// `docker run --rm -ti ubuntu:latest /lib64/ld-linux-x86-64.so.2 --list-diagnostics | grep path.system_dirs`.
 //
-//	docker run --rm -ti ubuntu /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 --help | grep -A6 "Shared library search path"
+// This yields the following ldconfig system search paths.
+//
+// TODO: Add other architectures that have custom `add_system_dir` macros (e.g. riscv)
+// TODO: Replace with executing the container's dynamlic linker with `--list-diagnostics`?
 func debianSystemSearchPaths() []string {
 	var paths []string
 	switch runtime.GOARCH {
@@ -328,6 +403,5 @@ func debianSystemSearchPaths() []string {
 		)
 	}
 	paths = append(paths, "/lib", "/usr/lib")
-
 	return paths
 }

internal/ldconfig/ldconfig_test.go

@@ -19,6 +19,7 @@ package ldconfig
 
 import (
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -124,3 +125,72 @@ include INCLUDED_PATTERN*
 		})
 	}
 }
+
+func TestAppendSystemSearchPathsToLdsoconf(t *testing.T) {
+	testCases := []struct {
+		description    string
+		initialContent string
+		noFile         bool
+		dirs           []string
+		expectedFinal  string
+		expectError    bool
+	}{
+		{
+			description:    "append to empty file",
+			initialContent: "",
+			dirs:           []string{"/lib", "/usr/lib"},
+			expectedFinal:  "/lib\n/usr/lib\n",
+		},
+		{
+			description:    "append to existing content",
+			initialContent: "# existing config\n/existing/path\n",
+			dirs:           []string{"/lib", "/usr/lib"},
+			expectedFinal:  "# existing config\n/existing/path\n/lib\n/usr/lib\n",
+		},
+		{
+			description:    "append empty does nothing",
+			initialContent: "# existing config\n",
+			dirs:           []string{},
+			expectedFinal:  "# existing config\n",
+		},
+		{
+			description: "append empty does not create file",
+			noFile:      true,
+			dirs:        []string{},
+		},
+		{
+			description: "append to non-existent file fails",
+			noFile:      true,
+			dirs:        []string{"/lib"},
+			expectError: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			tmpDir := t.TempDir()
+			configPath := filepath.Join(tmpDir, "ld.so.conf")
+
+			if !tc.noFile {
+				err := os.WriteFile(configPath, []byte(tc.initialContent), 0644)
+				require.NoError(t, err)
+			}
+
+			err := appendSystemSearchPathsToLdsoconf(configPath, tc.dirs...)
+			if tc.expectError {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			if !tc.noFile {
+				content, err := os.ReadFile(configPath)
+				require.NoError(t, err)
+				require.Equal(t, tc.expectedFinal, string(content))
+			} else {
+				_, err := os.Stat(configPath)
+				require.True(t, os.IsNotExist(err))
+			}
+		})
+	}
+}

tests/e2e/nvidia-container-toolkit_test.go

@@ -570,4 +570,23 @@ EOF`)
 			Expect(output).To(Equal(expectedOutput))
 		})
 	})
+
+	When("running a ubi9 container", Ordered, func() {
+		var (
+			expectedOutput string
+		)
+		BeforeAll(func(ctx context.Context) {
+			_, _, err := runner.Run(`docker pull redhat/ubi9`)
+			Expect(err).ToNot(HaveOccurred())
+
+			expectedOutput, _, err = runner.Run(`docker run --rm --runtime=runc redhat/ubi9 bash -c "ldconfig -p | grep libc.so."`)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("should include the system libraries when using the nvidia-container-runtime", func(ctx context.Context) {
+			output, _, err := runner.Run(`docker run --rm --runtime=nvidia -e NVIDIA_VISIBLE_DEVICES=all redhat/ubi9 bash -c "ldconfig -p | grep libc.so."`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output).To(Equal(expectedOutput))
+		})
+	})
 })