libct: speedup process.Env handling

libcontainer/env.go

@@ -0,0 +1,53 @@
+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) {
+	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"
@@ -185,8 +184,8 @@ func startInitialization() (retErr error) {
 		defer pidfdSocket.Close()
 	}
 
-	// clear the current process's environment to clean any libcontainer
-	// specific env vars.
+	// From here on, we don't need current process environment. It is not
+	// used directly anywhere below this point, but let's clear it anyway.
 	os.Clearenv()
 
 	defer func() {
@@ -209,9 +208,11 @@ func startInitialization() (retErr error) {
 }
 
 func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe *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
@@ -225,6 +226,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			pidfdSocket:   pidfdSocket,
 			config:        config,
 			logPipe:       logPipe,
+			addHome:       !homeSet,
 		}
 		return i.Init()
 	case initStandard:
@@ -236,36 +238,13 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			config:        config,
 			fifoFile:      fifoFile,
 			logPipe:       logPipe,
+			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 {
-		name, val, ok := strings.Cut(pair, "=")
-		if !ok {
-			return errors.New("invalid environment variable: missing '='")
-		}
-		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 {
@@ -294,8 +273,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
@@ -341,7 +320,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.
@@ -459,8 +438,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,
@@ -541,11 +521,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/bench_test.go

@@ -1,7 +1,10 @@
 package integration
 
 import (
+	"bytes"
+	"math/rand"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer"
@@ -27,9 +30,7 @@ func BenchmarkExecTrue(b *testing.B) {
 	_ = stdinR.Close()
 	defer func() {
 		_ = stdinW.Close()
-		if _, err := process.Wait(); err != nil {
-			b.Log(err)
-		}
+		waitProcess(process, b)
 	}()
 	ok(b, err)
 
@@ -42,10 +43,81 @@ func BenchmarkExecTrue(b *testing.B) {
 			LogLevel: "0", // Minimize forwardChildLogs involvement.
 		}
 		err := container.Run(exec)
-		if err != nil {
-			b.Fatal("exec failed:", err)
+		ok(b, err)
+		waitProcess(exec, b)
+	}
+	b.StopTimer()
+}
+
+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) {
+	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 func() {
+		_ = stdinW.Close()
+		waitProcess(process, b)
+	}()
+	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()
+		exec := &libcontainer.Process{
+			Cwd:    "/",
+			Args:   []string{"env"},
+			Env:    env,
+			Stdin:  buffers.Stdin,
+			Stdout: buffers.Stdout,
+			Stderr: buffers.Stderr,
+		}
+		err = container.Run(exec)
+		ok(b, err)
 		waitProcess(exec, b)
+		if !bytes.Equal(buffers.Stdout.Bytes(), wantOut.Bytes()) {
+			b.Fatalf("unexpected output: %s (stderr: %s)", buffers.Stdout, buffers.Stderr)
+		}
 	}
 	b.StopTimer()
 }

libcontainer/setns_init_linux.go

@@ -25,6 +25,7 @@ type linuxSetnsInit struct {
 	pidfdSocket   *os.File
 	config        *initConfig
 	logPipe       *os.File
+	addHome       bool
 }
 
 func (l *linuxSetnsInit) getSessionRingName() string {
@@ -100,7 +101,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 {
@@ -153,5 +154,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)
 }

libcontainer/standard_init_linux.go

@@ -27,6 +27,7 @@ type linuxStandardInit struct {
 	fifoFile      *os.File
 	logPipe       *os.File
 	config        *initConfig
+	addHome       bool
 }
 
 func (l *linuxStandardInit) getSessionRingParams() (string, uint32, uint32) {
@@ -189,7 +190,7 @@ func (l *linuxStandardInit) Init() error {
 			return err
 		}
 	}
-	if err := finalizeNamespace(l.config); err != nil {
+	if err := finalizeNamespace(l.config, l.addHome); err != nil {
 		return err
 	}
 	// finalizeNamespace can change user/group which clears the parent death
@@ -287,5 +288,5 @@ func (l *linuxStandardInit) 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)
 }