libct: speedup process.Env handling

libcontainer/env.go

@@ -0,0 +1,56 @@
+package libcontainer
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"slices"
+	"strings"
+)
+
+// prepareEnv checks supplied environment variables for validity, removes
+// duplicates (leaving the last value only), and sets PATH from env, if found.
+// Returns the deduplicated environment, and a flag telling if HOME is found.
+func prepareEnv(env []string) ([]string, bool, error) {
+	// Clear the current environment (better be safe than sorry).
+	os.Clearenv()
+
+	if env == nil {
+		return nil, false, nil
+	}
+	// Deduplication code based on dedupEnv from Go 1.22 os/exec.
+
+	// Construct the output in reverse order, to preserve the
+	// last occurrence of each key.
+	out := make([]string, 0, len(env))
+	saw := make(map[string]bool, len(env))
+	for n := len(env); n > 0; n-- {
+		kv := env[n-1]
+		i := strings.IndexByte(kv, '=')
+		if i == -1 {
+			return nil, false, errors.New("invalid environment variable: missing '='")
+		}
+		if i == 0 {
+			return nil, false, errors.New("invalid environment variable: name cannot be empty")
+		}
+		key := kv[:i]
+		if saw[key] { // Duplicate.
+			continue
+		}
+		saw[key] = true
+		if strings.IndexByte(kv, 0) >= 0 {
+			return nil, false, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key)
+		}
+		if key == "PATH" {
+			// Needs to be set as it is used for binary lookup.
+			if err := os.Setenv("PATH", kv[i+1:]); err != nil {
+				return nil, false, err
+			}
+		}
+		out = append(out, kv)
+	}
+	// Restore the original order.
+	slices.Reverse(out)
+
+	return out, saw["HOME"], nil
+}

libcontainer/env_test.go

@@ -0,0 +1,40 @@
+package libcontainer
+
+import (
+	"slices"
+	"testing"
+)
+
+func TestPrepareEnvDedup(t *testing.T) {
+	tests := []struct {
+		env, wantEnv []string
+	}{
+		{
+			env:     []string{},
+			wantEnv: []string{},
+		},
+		{
+			env:     []string{"HOME=/root", "FOO=bar"},
+			wantEnv: []string{"HOME=/root", "FOO=bar"},
+		},
+		{
+			env:     []string{"A=a", "A=b", "A=c"},
+			wantEnv: []string{"A=c"},
+		},
+		{
+			env:     []string{"TERM=vt100", "HOME=/home/one", "HOME=/home/two", "TERM=xterm", "HOME=/home/three", "FOO=bar"},
+			wantEnv: []string{"TERM=xterm", "HOME=/home/three", "FOO=bar"},
+		},
+	}
+
+	for _, tc := range tests {
+		env, _, err := prepareEnv(tc.env)
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		if !slices.Equal(env, tc.wantEnv) {
+			t.Errorf("want %v, got %v", tc.wantEnv, env)
+		}
+	}
+}

libcontainer/init_linux.go

@@ -11,7 +11,6 @@ import (
 	"runtime"
 	"runtime/debug"
 	"strconv"
-	"strings"
 	"syscall"
 
 	"github.com/containerd/console"
@@ -196,10 +195,6 @@ func startInitialization() (retErr error) {
 		dmzExe = os.NewFile(uintptr(dmzFd), "runc-dmz")
 	}
 
-	// clear the current process's environment to clean any libcontainer
-	// specific env vars.
-	os.Clearenv()
-
 	defer func() {
 		if err := recover(); err != nil {
 			if err2, ok := err.(error); ok {
@@ -220,9 +215,11 @@ func startInitialization() (retErr error) {
 }
 
 func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe, dmzExe *os.File) error {
-	if err := populateProcessEnvironment(config.Env); err != nil {
+	env, homeSet, err := prepareEnv(config.Env)
+	if err != nil {
 		return err
 	}
+	config.Env = env
 
 	// Clean the RLIMIT_NOFILE cache in go runtime.
 	// Issue: https://github.com/opencontainers/runc/issues/4195
@@ -237,6 +234,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			config:        config,
 			logPipe:       logPipe,
 			dmzExe:        dmzExe,
+			addHome:       !homeSet,
 		}
 		return i.Init()
 	case initStandard:
@@ -249,37 +247,13 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			fifoFile:      fifoFile,
 			logPipe:       logPipe,
 			dmzExe:        dmzExe,
+			addHome:       !homeSet,
 		}
 		return i.Init()
 	}
 	return fmt.Errorf("unknown init type %q", t)
 }
 
-// populateProcessEnvironment loads the provided environment variables into the
-// current processes's environment.
-func populateProcessEnvironment(env []string) error {
-	for _, pair := range env {
-		p := strings.SplitN(pair, "=", 2)
-		if len(p) < 2 {
-			return errors.New("invalid environment variable: missing '='")
-		}
-		name, val := p[0], p[1]
-		if name == "" {
-			return errors.New("invalid environment variable: name cannot be empty")
-		}
-		if strings.IndexByte(name, 0) >= 0 {
-			return fmt.Errorf("invalid environment variable %q: name contains nul byte (\\x00)", name)
-		}
-		if strings.IndexByte(val, 0) >= 0 {
-			return fmt.Errorf("invalid environment variable %q: value contains nul byte (\\x00)", name)
-		}
-		if err := os.Setenv(name, val); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 // verifyCwd ensures that the current directory is actually inside the mount
 // namespace root of the current process.
 func verifyCwd() error {
@@ -308,8 +282,8 @@ func verifyCwd() error {
 
 // finalizeNamespace drops the caps, sets the correct user
 // and working dir, and closes any leaked file descriptors
-// before executing the command inside the namespace
-func finalizeNamespace(config *initConfig) error {
+// before executing the command inside the namespace.
+func finalizeNamespace(config *initConfig, addHome bool) error {
 	// Ensure that all unwanted fds we may have accidentally
 	// inherited are marked close-on-exec so they stay out of the
 	// container
@@ -355,7 +329,7 @@ func finalizeNamespace(config *initConfig) error {
 	if err := system.SetKeepCaps(); err != nil {
 		return fmt.Errorf("unable to set keep caps: %w", err)
 	}
-	if err := setupUser(config); err != nil {
+	if err := setupUser(config, addHome); err != nil {
 		return fmt.Errorf("unable to setup user: %w", err)
 	}
 	// Change working directory AFTER the user has been set up, if we haven't done it yet.
@@ -473,8 +447,9 @@ func syncParentSeccomp(pipe *syncSocket, seccompFd int) error {
 	return readSync(pipe, procSeccompDone)
 }
 
-// setupUser changes the groups, gid, and uid for the user inside the container
-func setupUser(config *initConfig) error {
+// setupUser changes the groups, gid, and uid for the user inside the container,
+// and appends user's HOME to config.Env if addHome is true.
+func setupUser(config *initConfig, addHome bool) error {
 	// Set up defaults.
 	defaultExecUser := user.ExecUser{
 		Uid:  0,
@@ -555,11 +530,9 @@ func setupUser(config *initConfig) error {
 		return err
 	}
 
-	// if we didn't get HOME already, set it based on the user's HOME
-	if envHome := os.Getenv("HOME"); envHome == "" {
-		if err := os.Setenv("HOME", execUser.Home); err != nil {
-			return err
-		}
+	// If we didn't get HOME already, set it based on the user's HOME.
+	if addHome {
+		config.Env = append(config.Env, "HOME="+execUser.Home)
 	}
 	return nil
 }

libcontainer/integration/execin_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"math/rand"
 	"os"
 	"strconv"
 	"strings"
@@ -377,6 +378,80 @@ func TestExecInEnvironment(t *testing.T) {
 	}
 }
 
+func genBigEnv(count int) []string {
+	randStr := func(length int) string {
+		const charset = "abcdefghijklmnopqrstuvwxyz0123456789_"
+		b := make([]byte, length)
+		for i := range b {
+			b[i] = charset[rand.Intn(len(charset))]
+		}
+		return string(b)
+	}
+
+	envs := make([]string, count)
+	for i := 0; i < count; i++ {
+		key := strings.ToUpper(randStr(10))
+		value := randStr(20)
+		envs[i] = key + "=" + value
+	}
+
+	return envs
+}
+
+func BenchmarkExecInBigEnv(b *testing.B) {
+	if testing.Short() {
+		return
+	}
+	config := newTemplateConfig(b, nil)
+	container, err := newContainer(b, config)
+	ok(b, err)
+	defer destroyContainer(container)
+
+	// Execute a first process in the container
+	stdinR, stdinW, err := os.Pipe()
+	ok(b, err)
+	process := &libcontainer.Process{
+		Cwd:   "/",
+		Args:  []string{"cat"},
+		Env:   standardEnvironment,
+		Stdin: stdinR,
+		Init:  true,
+	}
+	err = container.Run(process)
+	_ = stdinR.Close()
+	defer stdinW.Close() //nolint: errcheck
+	ok(b, err)
+
+	const numEnv = 5000
+	env := append(standardEnvironment, genBigEnv(numEnv)...)
+	// Construct the expected output.
+	var wantOut bytes.Buffer
+	for _, e := range env {
+		wantOut.WriteString(e + "\n")
+	}
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		buffers := newStdBuffers()
+		process2 := &libcontainer.Process{
+			Cwd:    "/",
+			Args:   []string{"env"},
+			Env:    env,
+			Stdin:  buffers.Stdin,
+			Stdout: buffers.Stdout,
+			Stderr: buffers.Stderr,
+		}
+		err = container.Run(process2)
+		ok(b, err)
+		waitProcess(process2, b)
+		if !bytes.Equal(buffers.Stdout.Bytes(), wantOut.Bytes()) {
+			b.Fatalf("unexpected output: %s (stderr: %s)", buffers.Stdout, buffers.Stderr)
+		}
+	}
+	_ = stdinW.Close()
+	waitProcess(process, b)
+}
+
 func TestExecinPassExtraFiles(t *testing.T) {
 	if testing.Short() {
 		return

libcontainer/integration/template_test.go

@@ -32,7 +32,7 @@ type tParam struct {
 // and the default setup for devices.
 //
 // If p is nil, a default container is created.
-func newTemplateConfig(t *testing.T, p *tParam) *configs.Config {
+func newTemplateConfig(t testing.TB, p *tParam) *configs.Config {
 	var allowedDevices []*devices.Rule
 	for _, device := range specconv.AllowedDevices {
 		allowedDevices = append(allowedDevices, &device.Rule)

libcontainer/integration/utils_test.go

@@ -85,7 +85,7 @@ func ok(t testing.TB, err error) {
 	}
 }
 
-func waitProcess(p *libcontainer.Process, t *testing.T) {
+func waitProcess(p *libcontainer.Process, t testing.TB) {
 	t.Helper()
 	status, err := p.Wait()
 	if err != nil {
@@ -99,7 +99,7 @@ func waitProcess(p *libcontainer.Process, t *testing.T) {
 
 // newRootfs creates a new tmp directory and copies the busybox root
 // filesystem to it.
-func newRootfs(t *testing.T) string {
+func newRootfs(t testing.TB) string {
 	t.Helper()
 	dir := t.TempDir()
 	if err := copyBusybox(dir); err != nil {
@@ -165,7 +165,7 @@ func copyBusybox(dest string) error {
 	return nil
 }
 
-func newContainer(t *testing.T, config *configs.Config) (*libcontainer.Container, error) {
+func newContainer(t testing.TB, config *configs.Config) (*libcontainer.Container, error) {
 	name := strings.ReplaceAll(t.Name(), "/", "_") + strconv.FormatInt(-int64(time.Now().Nanosecond()), 35)
 	root := t.TempDir()
 
@@ -176,7 +176,7 @@ func newContainer(t *testing.T, config *configs.Config) (*libcontainer.Container
 //
 // buffers are returned containing the STDOUT and STDERR output for the run
 // along with the exit code and any go error
-func runContainer(t *testing.T, config *configs.Config, args ...string) (buffers *stdBuffers, exitCode int, err error) {
+func runContainer(t testing.TB, config *configs.Config, args ...string) (buffers *stdBuffers, exitCode int, err error) {
 	container, err := newContainer(t, config)
 	if err != nil {
 		return nil, -1, err
@@ -214,7 +214,7 @@ func runContainer(t *testing.T, config *configs.Config, args ...string) (buffers
 
 // runContainerOk is a wrapper for runContainer, simplifying its use for cases
 // when the run is expected to succeed and return exit code of 0.
-func runContainerOk(t *testing.T, config *configs.Config, args ...string) *stdBuffers {
+func runContainerOk(t testing.TB, config *configs.Config, args ...string) *stdBuffers {
 	buffers, exitCode, err := runContainer(t, config, args...)
 
 	t.Helper()

libcontainer/setns_init_linux.go

@@ -26,6 +26,7 @@ type linuxSetnsInit struct {
 	config        *initConfig
 	logPipe       *os.File
 	dmzExe        *os.File
+	addHome       bool
 }
 
 func (l *linuxSetnsInit) getSessionRingName() string {
@@ -101,7 +102,7 @@ func (l *linuxSetnsInit) Init() error {
 			return err
 		}
 	}
-	if err := finalizeNamespace(l.config); err != nil {
+	if err := finalizeNamespace(l.config, l.addHome); err != nil {
 		return err
 	}
 	if err := apparmor.ApplyProfile(l.config.AppArmorProfile); err != nil {
@@ -143,7 +144,7 @@ func (l *linuxSetnsInit) Init() error {
 
 	if l.dmzExe != nil {
 		l.config.Args[0] = name
-		return system.Fexecve(l.dmzExe.Fd(), l.config.Args, os.Environ())
+		return system.Fexecve(l.dmzExe.Fd(), l.config.Args, l.config.Env)
 	}
 	// Close all file descriptors we are not passing to the container. This is
 	// necessary because the execve target could use internal runc fds as the
@@ -163,5 +164,5 @@ func (l *linuxSetnsInit) Init() error {
 	if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil {
 		return err
 	}
-	return system.Exec(name, l.config.Args, os.Environ())
+	return system.Exec(name, l.config.Args, l.config.Env)
 }