From 127d585b05529d719afe43fbd4e6053c0516715c Mon Sep 17 00:00:00 2001 From: Marcus Gelderie Date: Mon, 14 Sep 2026 15:25:13 +0200 Subject: [PATCH] Fix --preserve-fds validation logic The --preserve-fds option can be used to pass extra file descriptors into the container. runc checks that those descriptors are open, but the check is ineffective: by the time it runs, runc itself (or the Go runtime) has already opened some file descriptors, which may reuse the very numbers that are supposed to be inherited from runc's caller. As a result, running runc with --preserve-fds set to 2 whilst not actually passing any descriptors reproducibly leaks runc-internal file descriptors into the container. Instead, an error message would be more helpful in this situation. Fix the check to look at the close-on-exec flag instead: a descriptor inherited from runc's caller can not have it set, as it would have been closed by execve, while those opened by runc and the Go runtime do have it set. Add integration tests for both runc run and runc exec. Closes: #5474 Signed-off-by: Marcus Gelderie Co-authored-by: Kir Kolyshkin Signed-off-by: Kir Kolyshkin --- tests/integration/exec.bats | 10 ++++++++++ tests/integration/run.bats | 5 +++++ utils_linux.go | 20 +++++++++++--------- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/tests/integration/exec.bats b/tests/integration/exec.bats index c9ad2f4860f..c29c6a791bf 100644 --- a/tests/integration/exec.bats +++ b/tests/integration/exec.bats @@ -154,6 +154,16 @@ function teardown() { assert_output "hello" } +@test "runc exec --preserve-fds with no inherited fd" { + run -0 runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + + # Use a separate shell so closing fd 3 does not interfere with bats itself. + # The check must happen before runc opens its log file in fd 3 and mistakes + # that descriptor for one inherited from runc's caller. + run ! bash -c 'exec 3>&-; exec "$@"' bash runc --log preserve-fds.log exec --preserve-fds=1 test_busybox true + assert_output --partial "preserved-fd 0" +} + function check_exec_debug() { [[ "$*" == *"nsexec container setup"* ]] [[ "$*" == *"child process in init()"* ]] diff --git a/tests/integration/run.bats b/tests/integration/run.bats index a8d0b4e6abb..284ddee0aeb 100644 --- a/tests/integration/run.bats +++ b/tests/integration/run.bats @@ -17,6 +17,11 @@ function teardown() { run ! runc state test_hello } +@test "runc run --preserve-fds with no inherited fd" { + run ! bash -c 'exec 3>&-; exec "$@"' bash runc run --preserve-fds=1 test_missing_fd + assert_output --partial "preserved-fd 0" +} + @test "runc run --keep" { run -0 runc run --keep test_run_keep diff --git a/utils_linux.go b/utils_linux.go index 1da1c872d88..1cd2f4ce395 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -15,7 +15,6 @@ import ( "github.com/urfave/cli/v3" "golang.org/x/sys/unix" - "github.com/opencontainers/runc/internal/pathrs" "github.com/opencontainers/runc/internal/third_party/systemd/activation" "github.com/opencontainers/runc/libcontainer" "github.com/opencontainers/runc/libcontainer/configs" @@ -245,16 +244,19 @@ func (r *runner) run(config *specs.Process) (_ int, retErr error) { process.ExtraFiles = append(process.ExtraFiles, r.listenFDs...) } baseFd := 3 + len(process.ExtraFiles) - procSelfFd, closer, err := pathrs.ProcThreadSelfOpen("fd/", unix.O_DIRECTORY|unix.O_CLOEXEC) - if err != nil { - return -1, err - } - defer closer() - defer procSelfFd.Close() for i := baseFd; i < baseFd+r.preserveFDs; i++ { - err := unix.Faccessat(int(procSelfFd.Fd()), strconv.Itoa(i), unix.F_OK, 0) + + // Check that the fd was really inherited from runc's caller. Merely + // checking that the fd is open is not sufficient, as the fd number + // could have been reused by runc itself (or the Go runtime). Any such + // fd has the close-on-exec flag set, while an inherited one can not + // have it, as it would have been closed by execve. + flags, err := unix.FcntlInt(uintptr(i), unix.F_GETFD, 0) if err != nil { - return -1, fmt.Errorf("unable to stat preserved-fd %d (of %d): %w", i-baseFd, r.preserveFDs, err) + return -1, fmt.Errorf("fcntl on preserved-fd %d (of %d) failed: %w", i-baseFd, r.preserveFDs, err) + } + if flags&unix.FD_CLOEXEC != 0 { + return -1, fmt.Errorf("preserved-fd %d (of %d) has the close-on-exec flag set", i-baseFd, r.preserveFDs) } process.ExtraFiles = append(process.ExtraFiles, os.NewFile(uintptr(i), "PreserveFD:"+strconv.Itoa(i))) }