From fa722c1d17867cf24989bad42a59dfc6e00269e3 Mon Sep 17 00:00:00 2001 From: Jaroslav Jindrak Date: Fri, 20 Jan 2023 21:41:12 +0100 Subject: [PATCH 1/2] libcontainer: skip chown of /dev/null caused by fd redirection In 18c4760a (libct: fixStdioPermissions: skip chown if not needed) the check whether the STDIO file descriptors point to /dev/null was removed which can cause /dev/null to change ownership e.g. when using docker exec on a running container: $ ls -l /dev/null crw-rw-rw- 1 root root 1, 3 Aug 1 14:12 /dev/null $ docker exec -u test 0ad6d3064e9d ls $ ls -l /dev/null crw-rw-rw- 1 test root 1, 3 Aug 1 14:12 /dev/null Signed-off-by: Jaroslav Jindrak (cherry picked from commit 7e5e017dbab0f6ecb2c4b63c29c5ac44fc4e6ec6) Signed-off-by: Kir Kolyshkin --- libcontainer/init_linux.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 1e5c394c3e0..2e4c59353c8 100644 --- 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 } From 9233b3d059088a7bd35aa97a8ccd6de98ec90702 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 2 Feb 2023 13:07:56 -0800 Subject: [PATCH 2/2] tests/int: test for /dev/null owner regression Signed-off-by: Kir Kolyshkin (cherry picked from commit 1bb6209aa1dac56b193d3947bc54292cbd8151cd) Signed-off-by: Kir Kolyshkin --- tests/integration/exec.bats | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/integration/exec.bats b/tests/integration/exec.bats index 140cd181011..47c047bd638 100644 --- 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