Merge pull request #3731 from kolyshkin/1.1-fix-dev-null

AkihiroSuda · web-flow · commit 3775df9fcb78 · 2023-02-10T04:56:33.000+09:00
[1.1] libcontainer: skip chown of /dev/null caused by fd redirection
diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go
@@ -411,8 +411,9 @@ func fixStdioPermissions(u *user.ExecUser) error {
 			return &os.PathError{Op: "fstat", Path: file.Name(), Err: err}
 		}
 
-		// Skip chown if uid is already the one we want.
-		if int(s.Uid) == u.Uid {
+		// Skip chown if uid is already the one we want or any of the STDIO descriptors
+		// were redirected to /dev/null.
+		if int(s.Uid) == u.Uid || s.Rdev == null.Rdev {
 			continue
 		}
 
diff --git a/tests/integration/exec.bats b/tests/integration/exec.bats
@@ -125,10 +125,25 @@ function teardown() {
 
 	runc exec --user 1000:1000 test_busybox id
 	[ "$status" -eq 0 ]
-
 	[[ "${output}" == "uid=1000 gid=1000"* ]]
 }
 
+# https://github.com/opencontainers/runc/issues/3674.
+@test "runc exec --user vs /dev/null ownership" {
+	requires root
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 0 ]
+
+	ls -l /dev/null
+	__runc exec -d --user 1000:1000 test_busybox id </dev/null
+	ls -l /dev/null
+	UG=$(stat -c %u:%g /dev/null)
+
+	# Host's /dev/null must be owned by root.
+	[ "$UG" = "0:0" ]
+}
+
 @test "runc exec --additional-gids" {
 	requires root
 

Original file line number	Diff line number	Diff line change
`@@ -411,8 +411,9 @@ func fixStdioPermissions(u *user.ExecUser) error {`
`411`	`411`	`return &os.PathError{Op: "fstat", Path: file.Name(), Err: err}`
`412`	`412`	`}`
`413`	`413`
`414`		`- // Skip chown if uid is already the one we want.`
`415`		`- if int(s.Uid) == u.Uid {`
	`414`	`+ // Skip chown if uid is already the one we want or any of the STDIO descriptors`
	`415`	`+ // were redirected to /dev/null.`
	`416`	`+ if int(s.Uid) == u.Uid \|\| s.Rdev == null.Rdev {`
`416`	`417`	`continue`
`417`	`418`	`}`
`418`	`419`