runc exec: fix RLIMIT_NOFILE race

libcontainer/process_linux.go

@@ -268,20 +268,24 @@ func (p *setnsProcess) start() (retErr error) {
 			}
 		}
 	}
-	// set rlimits, this has to be done here because we lose permissions
-	// to raise the limits once we enter a user-namespace
-	if err := setupRlimits(p.config.Rlimits, p.pid()); err != nil {
-		return fmt.Errorf("error setting rlimits for process: %w", err)
-	}
 	if err := utils.WriteJSON(p.comm.initSockParent, p.config); err != nil {
 		return fmt.Errorf("error writing config to pipe: %w", err)
 	}
 
 	ierr := parseSync(p.comm.syncSockParent, func(sync *syncT) error {
 		switch sync.Type {
 		case procReady:
-			// This shouldn't happen.
-			panic("unexpected procReady in setns")
+			// Set rlimits here because we lose permissions
+			// to raise the limits once we enter a user-namespace.
+			// Also, Go runtime sets its own RLIMIT_NOFILE on start
+			// so we need to reset it here.
+			if err := setupRlimits(p.config.Rlimits, p.pid()); err != nil {
+				return fmt.Errorf("error setting rlimits for process: %w", err)
+			}
+			// Sync with child.
+			if err := writeSync(p.comm.syncSockParent, procRun); err != nil {
+				return err
+			}
 		case procHooks:
 			// This shouldn't happen.
 			panic("unexpected procHooks in setns")

libcontainer/setns_init_linux.go

@@ -81,6 +81,14 @@ func (l *linuxSetnsInit) Init() error {
 		return err
 	}
 	defer selinux.SetExecLabel("") //nolint: errcheck
+
+	// Tell our parent that we're ready to Execv. This must be done before the
+	// Seccomp rules have been applied, because we need to be able to read and
+	// write to a socket.
+	if err := syncParentReady(l.pipe); err != nil {
+		return fmt.Errorf("sync ready: %w", err)
+	}
+
 	// Without NoNewPrivileges seccomp is a privileged operation, so we need to
 	// do this before dropping capabilities; otherwise do it as late as possible
 	// just before execve so as few syscalls take place after it as possible.

tests/integration/exec.bats

@@ -476,3 +476,19 @@ EOF
 	[ "$status" -eq 0 ]
 	[[ "${lines[0]}" == "Cpus_allowed_list:	$all_cpus" ]]
 }
+
+@test "runc exec with RLIMIT_NOFILE" {
+	update_config '.process.capabilities.bounding = ["CAP_SYS_RESOURCE"]'
+	update_config '.process.rlimits = [{"type": "RLIMIT_NOFILE", "hard": 65536, "soft": 65536}]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+
+	update_config '.process.args = ["/bin/sh", "-c", "ulimit -n"]'
+	runc run test_ulimit
+	[[ "${output}" == "65536" ]]
+
+	# issue: https://github.com/opencontainers/runc/issues/4195
+	runc exec test_busybox /bin/sh -c "ulimit -n"
+	[[ "${output}" == "65536" ]]
+}

tests/integration/ulimit.bats

@@ -0,0 +1,51 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function setup() {
+	setup_busybox
+}
+
+function teardown() {
+	teardown_bundle
+}
+
+@test "runc run with RLIMIT_NOFILE" {
+	update_config '.process.rlimits = [{"type": "RLIMIT_NOFILE", "hard": 65536, "soft": 65536}]'
+
+	update_config '.process.args = ["/bin/sh", "-c", "ulimit -n"]'
+	runc run test_ulimit
+	[ "$status" -eq 0 ]
+	[[ "${output}" == "65536" ]]
+}
+
+@test "runc exec with RLIMIT_NOFILE" {
+	update_config '	 .process.rlimits = [{"type": "RLIMIT_NOFILE", "hard": 2000, "soft": 1000}]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" nofile
+	[ "$status" -eq 0 ]
+
+	for ((i = 0; i < 100; i++)); do
+		runc exec nofile sh -c 'ulimit -n'
+		echo "[$i] $output"
+		[[ "${output}" == "1000" ]]
+	done
+}
+
+@test "runc run+exec two containers with RLIMIT_NOFILE" {
+	update_config '.process.capabilities.bounding = ["CAP_SYS_RESOURCE"]'
+	update_config '.process.rlimits = [{"type": "RLIMIT_NOFILE", "hard": 65536, "soft": 65536}]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+
+	update_config '.process.args = ["/bin/sh", "-c", "ulimit -n"]'
+	runc run test_ulimit
+	[ "$status" -eq 0 ]
+	[[ "${output}" == "65536" ]]
+
+	# issue: https://github.com/opencontainers/runc/issues/4195
+	runc exec test_busybox /bin/sh -c "ulimit -n"
+	[ "$status" -eq 0 ]
+	[[ "${output}" == "65536" ]]
+}