Merge pull request #5034 from lifubang/ci-detect-fdleak-try-best-1.4

[1.4] fix fd leaks and detect them as comprehensively as possible

Author: AkihiroSuda

go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/checkpoint-restore/go-criu/v7 v7.2.0
 	github.com/containerd/console v1.0.5
 	github.com/coreos/go-systemd/v22 v22.6.0
-	github.com/cyphar/filepath-securejoin v0.6.0
+	github.com/cyphar/filepath-securejoin v0.6.1
 	github.com/docker/go-units v0.5.0
 	github.com/godbus/dbus/v5 v5.1.0
 	github.com/moby/sys/capability v0.4.0

go.sum

@@ -11,8 +11,8 @@ github.com/coreos/go-systemd/v22 v22.6.0 h1:aGVa/v8B7hpb0TKl0MWoAavPDmHvobFe5R5z
 github.com/coreos/go-systemd/v22 v22.6.0/go.mod h1:iG+pp635Fo7ZmV/j14KUcmEyWF+0X7Lua8rrTWzYgWU=
 github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo=
 github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
-github.com/cyphar/filepath-securejoin v0.6.0 h1:BtGB77njd6SVO6VztOHfPxKitJvd/VPT+OFBFMOi1Is=
-github.com/cyphar/filepath-securejoin v0.6.0/go.mod h1:A8hd4EnAeyujCJRrICiOWqjS1AX0a9kM5XL+NwKoYSc=
+github.com/cyphar/filepath-securejoin v0.6.1 h1:5CeZ1jPXEiYt3+Z6zqprSAgSWiggmpVyciv8syjIpVE=
+github.com/cyphar/filepath-securejoin v0.6.1/go.mod h1:A8hd4EnAeyujCJRrICiOWqjS1AX0a9kM5XL+NwKoYSc=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

libcontainer/rootfs_linux.go

@@ -589,6 +589,12 @@ func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) {
 
 func mountToRootfs(c *mountConfig, m mountEntry) error {
 	rootfs := c.root
+	defer func() {
+		if m.dstFile != nil {
+			_ = m.dstFile.Close()
+			m.dstFile = nil
+		}
+	}()
 
 	// procfs and sysfs are special because we need to ensure they are actually
 	// mounted on a specific path in a container without any funny business.
@@ -629,12 +635,6 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 	if err := m.createOpenMountpoint(rootfs); err != nil {
 		return fmt.Errorf("create mountpoint for %s mount: %w", m.Destination, err)
 	}
-	defer func() {
-		if m.dstFile != nil {
-			_ = m.dstFile.Close()
-			m.dstFile = nil
-		}
-	}()
 
 	switch m.Device {
 	case "mqueue":
@@ -995,6 +995,8 @@ func createDeviceNode(rootfs string, node *devices.Device, bind bool) error {
 	if err != nil {
 		return fmt.Errorf("mkdir parent of device inode %q: %w", node.Path, err)
 	}
+	defer destDir.Close()
+
 	if bind {
 		return bindMountDeviceNode(destDir, destName, node)
 	}

tests/integration/create.bats

@@ -10,6 +10,70 @@ function teardown() {
 	teardown_bundle
 }
 
+# is_allowed_fdtarget checks whether the target of a file descriptor symlink
+# conforms to the allowed whitelist.
+#
+# This whitelist reflects the set of file descriptors that runc legitimately
+# opens during container lifecycle operations (e.g., exec, create, and run).
+# If runc's internal behavior changes (e.g., new FD types are introduced),
+# this function MUST be updated accordingly to avoid false positives.
+#
+is_allowed_fdtarget() {
+	local target="$1"
+	{
+		# pty devices for stdio
+		grep -Ex "/dev/pts/[0-9]+" <<<"$target" ||
+			# eventfd, eventpoll, signalfd, etc.
+			grep -Ex "anon_inode:\[.+\]" <<<"$target" ||
+			# procfs handle cache (pathrs-lite / libpathrs)
+			grep -Ex "/(proc)?" <<<"$target" ||
+			# anonymous sockets used for IPC
+			grep -Ex "socket:\[[0-9]+\]" <<<"$target" ||
+			# anonymous pipes used for I/O forwarding
+			grep -Ex "pipe:\[[0-9]+\]" <<<"$target" ||
+			# "runc start" synchronisation barrier FIFO
+			grep -Ex ".*/exec\.fifo" <<<"$target" ||
+			# temporary internal fd used in exec.fifo FIFO reopen (pathrs-lite / libpathrs)
+			grep -Ex "(/proc)?/1/task/1/fd" <<<"$target" ||
+			# overlayfs binary reference (CVE-2019-5736)
+			grep -Ex "/runc" <<<"$target" ||
+			# memfd cloned binary (CVE-2019-5736)
+			grep -Fx "/memfd:runc_cloned:/proc/self/exe (deleted)" <<<"$target"
+	} >/dev/null
+	return "$?"
+}
+
+@test "runc create[detect fd leak as comprehensively as possible]" {
+	runc create --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+
+	testcontainer test_busybox created
+
+	pid=$(__runc state test_busybox | jq '.pid')
+	violation_found=0
+
+	while IFS= read -rd '' link; do
+		fd_name=$(basename "$link")
+		# Skip . and ..
+		if [[ "$fd_name" == "." || "$fd_name" == ".." ]]; then
+			continue
+		fi
+
+		# Resolve symlink target (use readlink)
+		target=$(readlink "$link" 2>/dev/null)
+		if [[ -z "$target" ]]; then
+			echo "Warning: Cannot read target of $link"
+			continue
+		fi
+
+		if ! is_allowed_fdtarget "$target"; then
+			echo "Violation: FD $fd_name -> '$target'"
+			violation_found=1
+		fi
+	done < <(find "/proc/$pid/fd" -type l -print0)
+	[ "$violation_found" -eq 0 ]
+}
+
 @test "runc create" {
 	runc create --console-socket "$CONSOLE_SOCKET" test_busybox
 	[ "$status" -eq 0 ]

tests/integration/seccomp.bats

@@ -185,3 +185,16 @@ function flags_value() {
 	[[ "$output" == *"error running startContainer hook"* ]]
 	[[ "$output" == *"bad system call"* ]]
 }
+
+@test "runc run [seccomp] (verify syscall compatibility after seccomp enforcement)" {
+	update_config '   .process.args = ["true"]
+			| .process.noNewPrivileges = false
+			| .linux.seccomp = {
+				"defaultAction":"SCMP_ACT_ALLOW",
+				"architectures":["SCMP_ARCH_X86","SCMP_ARCH_X32","SCMP_ARCH_X86_64","SCMP_ARCH_AARCH64","SCMP_ARCH_ARM"],
+				"syscalls":[{"names":["close_range", "fsopen", "fsconfig", "fspick", "openat2", "open_tree", "move_mount", "mount_setattr"], "action":"SCMP_ACT_ERRNO", "errnoRet": 38}]
+			}'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+}