From de6637c27cffb5e92abb7723ae606f8940c4d408 Mon Sep 17 00:00:00 2001 From: Jimmy Tran Date: Fri, 7 Feb 2025 11:19:11 -0800 Subject: [PATCH] Recompute `max` variable after setting FD in the bitmap. `fdBitmap.FirstZero()` could return `max` value; if it does, then recompute the max value to avoid reusing the old max value twice. The default bitmap size for file descriptors in gVisor is 65535. Add a pipe test that attempts to create more than 65535 FDs to hit the edge case where fdBitmap.FirstZero() returns the default bitmap max value of 65535. TESTED: http://sponge2/4c12ce75-3763-4773-ad62-87c6b8fe0446 http://sponge2/9c9d6ea0-b69c-432c-a16b-9446214109ba PiperOrigin-RevId: 724410846 --- pkg/sentry/kernel/fd_table.go | 3 +++ test/syscalls/linux/pipe.cc | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkg/sentry/kernel/fd_table.go b/pkg/sentry/kernel/fd_table.go index 3eb0ddf69..7277bead7 100644 --- a/pkg/sentry/kernel/fd_table.go +++ b/pkg/sentry/kernel/fd_table.go @@ -267,6 +267,9 @@ func (f *FDTable) NewFDs(ctx context.Context, minFD int32, files []*vfs.FileDesc break } f.fdBitmap.Add(fd) + if fd == uint32(max) { + max = int32(fd + 1) + } if df := f.set(int32(fd), files[len(fds)], flags); df != nil { panic("file set") } diff --git a/test/syscalls/linux/pipe.cc b/test/syscalls/linux/pipe.cc index a4be75908..3fc252208 100644 --- a/test/syscalls/linux/pipe.cc +++ b/test/syscalls/linux/pipe.cc @@ -691,6 +691,31 @@ TEST_P(PipeTest, ZeroSize) { ASSERT_THAT(read(rfd_.get(), nullptr, 0), SyscallSucceedsWithValue(0)); } +// Test that we can open more FDs than the max default value without crashing. +TEST_P(PipeTest, PipeFdCount) { + SKIP_IF(!CreateBlocking()); + + // We make too many calls to go through full save cycles. + DisableSave ds; + constexpr size_t kMaxFd = 66000; + std::vector fds; + + while (true) { + int pipefd[2]; + ASSERT_THAT(pipe2(pipefd, 0), SyscallSucceeds()); + ASSERT_NE(pipefd[0], pipefd[1]); + fds.push_back(pipefd[0]); + fds.push_back(pipefd[1]); + if (static_cast(pipefd[1]) > kMaxFd) { + break; + } + } + + for (const auto fd : fds) { + close(fd); + } +} + std::string PipeCreatorName(::testing::TestParamInfo info) { return info.param.name_; // Use the name specified. }