Skip to content

Commit 2abe126

Browse files
committed
Only allow host-relative LDConfig paths
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]>
1 parent c90338d commit 2abe126

File tree

7 files changed

+240
-47
lines changed

7 files changed

+240
-47
lines changed

internal/config/cli.go

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package config
1818

1919
import (
20+
"fmt"
2021
"os"
2122
"strings"
2223
)
@@ -34,29 +35,62 @@ type ContainerCLIConfig struct {
3435
NoPivot bool `toml:"no-pivot,omitempty"`
3536
NoCgroups bool `toml:"no-cgroups"`
3637
User string `toml:"user"`
37-
Ldconfig string `toml:"ldconfig"`
38+
// Ldconfig represents the path to the ldconfig binary to be used to update
39+
// the ldcache in a container as it is being created.
40+
// If this path starts with a '@' the path is relative to the host and if
41+
// not it is treated as a container path.
42+
//
43+
// Note that the use of container paths are disabled by default and if this
44+
// is required, the features.allow-ldconfig-from-container feature gate must
45+
// be enabled explicitly.
46+
Ldconfig ldconfigPath `toml:"ldconfig"`
3847
}
3948

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

47-
// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
56+
// An ldconfigPath is used to represent the path to ldconfig.
57+
type ldconfigPath string
58+
59+
func (p ldconfigPath) assertValid(allowContainerRelativePath bool) error {
60+
if p.isHostRelative() {
61+
return nil
62+
}
63+
if allowContainerRelativePath {
64+
return nil
65+
}
66+
return fmt.Errorf("nvidia-container-cli.ldconfig value %q is not host-relative (does not start with a '@')", p)
67+
}
68+
69+
func (p ldconfigPath) isHostRelative() bool {
70+
return strings.HasPrefix(string(p), "@")
71+
}
72+
73+
// normalize returns the resolved path of the configured LDConfig binary.
4874
// This is only done for host LDConfigs and is required to handle systems where
4975
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
50-
func NormalizeLDConfigPath(path string) string {
51-
if !strings.HasPrefix(path, "@") {
52-
return path
76+
func (p ldconfigPath) normalize() ldconfigPath {
77+
if !p.isHostRelative() {
78+
return p
5379
}
5480

81+
path := string(p)
5582
trimmedPath := strings.TrimSuffix(strings.TrimPrefix(path, "@"), ".real")
5683
// If the .real path exists, we return that.
5784
if _, err := os.Stat(trimmedPath + ".real"); err == nil {
58-
return "@" + trimmedPath + ".real"
85+
return ldconfigPath("@" + trimmedPath + ".real")
5986
}
6087
// If the .real path does not exists (or cannot be read) we return the non-.real path.
61-
return "@" + trimmedPath
88+
return ldconfigPath("@" + trimmedPath)
89+
}
90+
91+
// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
92+
// This is only done for host LDConfigs and is required to handle systems where
93+
// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
94+
func NormalizeLDConfigPath(path string) string {
95+
return string(ldconfigPath(path).normalize())
6296
}

internal/config/cli_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestNormalizeLDConfigPath(t *testing.T) {
3333

3434
testCases := []struct {
3535
description string
36-
ldconfig string
36+
ldconfig ldconfigPath
3737
expected string
3838
}{
3939
{
@@ -51,12 +51,12 @@ func TestNormalizeLDConfigPath(t *testing.T) {
5151
},
5252
{
5353
description: "host .real file exists is returned",
54-
ldconfig: "@" + filepath.Join(testDir, "exists.real"),
54+
ldconfig: ldconfigPath("@" + filepath.Join(testDir, "exists.real")),
5555
expected: "@" + filepath.Join(testDir, "exists.real"),
5656
},
5757
{
5858
description: "host resolves .real file",
59-
ldconfig: "@" + filepath.Join(testDir, "exists"),
59+
ldconfig: ldconfigPath("@" + filepath.Join(testDir, "exists")),
6060
expected: "@" + filepath.Join(testDir, "exists.real"),
6161
},
6262
{

internal/config/config.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package config
1818

1919
import (
2020
"bufio"
21+
"errors"
2122
"os"
2223
"path/filepath"
2324
"strings"
@@ -51,6 +52,8 @@ var (
5152
NVIDIAContainerToolkitExecutable = "nvidia-container-toolkit"
5253
)
5354

55+
var errInvalidConfig = errors.New("invalid config value")
56+
5457
// Config represents the contents of the config.toml file for the NVIDIA Container Toolkit
5558
// Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go
5659
type Config struct {
@@ -127,8 +130,20 @@ func GetDefault() (*Config, error) {
127130
return &d, nil
128131
}
129132

130-
func getLdConfigPath() string {
131-
return NormalizeLDConfigPath("@/sbin/ldconfig")
133+
// assertValid checks for a valid config.
134+
func (c *Config) assertValid() error {
135+
err := c.NVIDIAContainerCLIConfig.Ldconfig.assertValid(c.Features.AllowLDConfigFromContainer.IsEnabled())
136+
if err != nil {
137+
return errors.Join(err, errInvalidConfig)
138+
}
139+
return nil
140+
}
141+
142+
// getLdConfigPath allows us to override this function for testing.
143+
var getLdConfigPath = getLdConfigPathStub
144+
145+
func getLdConfigPathStub() ldconfigPath {
146+
return ldconfigPath("@/sbin/ldconfig").normalize()
132147
}
133148

134149
func getUserGroup() string {

0 commit comments

Comments
 (0)