Skip to content

Commit

Permalink
Only allow host-relative LDConfig paths
Browse files Browse the repository at this point in the history
This change only allows host-relative LDConfig paths.

An allow-ldconfig-from-container feature flag is added to allow for this
the default behaviour to be changed.

Signed-off-by: Evan Lezar <[email protected]>
  • Loading branch information
elezar committed Nov 26, 2024
1 parent c90338d commit 2abe126
Show file tree
Hide file tree
Showing 7 changed files with 240 additions and 47 deletions.
50 changes: 42 additions & 8 deletions internal/config/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package config

import (
"fmt"
"os"
"strings"
)
Expand All @@ -34,29 +35,62 @@ type ContainerCLIConfig struct {
NoPivot bool `toml:"no-pivot,omitempty"`
NoCgroups bool `toml:"no-cgroups"`
User string `toml:"user"`
Ldconfig string `toml:"ldconfig"`
// Ldconfig represents the path to the ldconfig binary to be used to update
// the ldcache in a container as it is being created.
// If this path starts with a '@' the path is relative to the host and if
// not it is treated as a container path.
//
// Note that the use of container paths are disabled by default and if this
// is required, the features.allow-ldconfig-from-container feature gate must
// be enabled explicitly.
Ldconfig ldconfigPath `toml:"ldconfig"`
}

// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func (c *ContainerCLIConfig) NormalizeLDConfigPath() string {
return NormalizeLDConfigPath(c.Ldconfig)
return string(c.Ldconfig.normalize())
}

// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
// An ldconfigPath is used to represent the path to ldconfig.
type ldconfigPath string

func (p ldconfigPath) assertValid(allowContainerRelativePath bool) error {
if p.isHostRelative() {
return nil
}
if allowContainerRelativePath {
return nil
}
return fmt.Errorf("nvidia-container-cli.ldconfig value %q is not host-relative (does not start with a '@')", p)
}

func (p ldconfigPath) isHostRelative() bool {
return strings.HasPrefix(string(p), "@")
}

// normalize returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func NormalizeLDConfigPath(path string) string {
if !strings.HasPrefix(path, "@") {
return path
func (p ldconfigPath) normalize() ldconfigPath {
if !p.isHostRelative() {
return p
}

path := string(p)
trimmedPath := strings.TrimSuffix(strings.TrimPrefix(path, "@"), ".real")
// If the .real path exists, we return that.
if _, err := os.Stat(trimmedPath + ".real"); err == nil {
return "@" + trimmedPath + ".real"
return ldconfigPath("@" + trimmedPath + ".real")
}
// If the .real path does not exists (or cannot be read) we return the non-.real path.
return "@" + trimmedPath
return ldconfigPath("@" + trimmedPath)
}

// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
// This is only done for host LDConfigs and is required to handle systems where
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
func NormalizeLDConfigPath(path string) string {
return string(ldconfigPath(path).normalize())
}
6 changes: 3 additions & 3 deletions internal/config/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestNormalizeLDConfigPath(t *testing.T) {

testCases := []struct {
description string
ldconfig string
ldconfig ldconfigPath
expected string
}{
{
Expand All @@ -51,12 +51,12 @@ func TestNormalizeLDConfigPath(t *testing.T) {
},
{
description: "host .real file exists is returned",
ldconfig: "@" + filepath.Join(testDir, "exists.real"),
ldconfig: ldconfigPath("@" + filepath.Join(testDir, "exists.real")),
expected: "@" + filepath.Join(testDir, "exists.real"),
},
{
description: "host resolves .real file",
ldconfig: "@" + filepath.Join(testDir, "exists"),
ldconfig: ldconfigPath("@" + filepath.Join(testDir, "exists")),
expected: "@" + filepath.Join(testDir, "exists.real"),
},
{
Expand Down
19 changes: 17 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package config

import (
"bufio"
"errors"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -51,6 +52,8 @@ var (
NVIDIAContainerToolkitExecutable = "nvidia-container-toolkit"
)

var errInvalidConfig = errors.New("invalid config value")

// Config represents the contents of the config.toml file for the NVIDIA Container Toolkit
// Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go
type Config struct {
Expand Down Expand Up @@ -127,8 +130,20 @@ func GetDefault() (*Config, error) {
return &d, nil
}

func getLdConfigPath() string {
return NormalizeLDConfigPath("@/sbin/ldconfig")
// assertValid checks for a valid config.
func (c *Config) assertValid() error {
err := c.NVIDIAContainerCLIConfig.Ldconfig.assertValid(c.Features.AllowLDConfigFromContainer.IsEnabled())
if err != nil {
return errors.Join(err, errInvalidConfig)
}
return nil
}

// getLdConfigPath allows us to override this function for testing.
var getLdConfigPath = getLdConfigPathStub

func getLdConfigPathStub() ldconfigPath {
return ldconfigPath("@/sbin/ldconfig").normalize()
}

func getUserGroup() string {
Expand Down
Loading

0 comments on commit 2abe126

Please sign in to comment.