From 7f8de3bf92decbd745a4bc4e8aebf1ba1159ed4b Mon Sep 17 00:00:00 2001 From: Zach Koopmans Date: Thu, 10 Jan 2019 09:43:43 -0800 Subject: Fixing select call to not enforce RLIMIT_NOFILE. Removing check to RLIMIT_NOFILE in select call. Adding unit test to select suite to document behavior. Moving setrlimit class from mlock to a util file for reuse. Fixing flaky test based on comments from Jamie. PiperOrigin-RevId: 228726131 Change-Id: Ie9dbe970bbf835ba2cca6e17eec7c2ee6fadf459 --- test/syscalls/linux/BUILD | 6 +++++ test/syscalls/linux/mlock.cc | 19 +++---------- test/syscalls/linux/select.cc | 62 +++++++++++++++++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 27 deletions(-) (limited to 'test/syscalls/linux') diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 028c686a8..a8a6e15ee 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1033,6 +1033,7 @@ cc_binary( "//test/util:cleanup", "//test/util:memory_util", "//test/util:multiprocess_util", + "//test/util:rlimit_util", "//test/util:test_main", "//test/util:test_util", "@com_google_googletest//:gtest", @@ -1650,6 +1651,11 @@ cc_binary( linkstatic = 1, deps = [ ":base_poll_test", + "//test/util:file_descriptor", + "//test/util:multiprocess_util", + "//test/util:posix_error", + "//test/util:rlimit_util", + "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", "@com_google_absl//absl/time", diff --git a/test/syscalls/linux/mlock.cc b/test/syscalls/linux/mlock.cc index 1d93bff58..a492b2404 100644 --- a/test/syscalls/linux/mlock.cc +++ b/test/syscalls/linux/mlock.cc @@ -12,18 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include -#include #include #include #include #include +#include +#include #include "gmock/gmock.h" #include "test/util/capability_util.h" #include "test/util/cleanup.h" #include "test/util/memory_util.h" #include "test/util/multiprocess_util.h" +#include "test/util/rlimit_util.h" #include "test/util/test_util.h" using ::testing::_; @@ -58,20 +59,6 @@ bool IsPageMlocked(uintptr_t addr) { return true; } -PosixErrorOr ScopedSetSoftRlimit(int resource, rlim_t newval) { - struct rlimit old_rlim; - if (getrlimit(resource, &old_rlim) != 0) { - return PosixError(errno, "getrlimit failed"); - } - struct rlimit new_rlim = old_rlim; - new_rlim.rlim_cur = newval; - if (setrlimit(resource, &new_rlim) != 0) { - return PosixError(errno, "setrlimit failed"); - } - return Cleanup([resource, old_rlim] { - TEST_PCHECK(setrlimit(resource, &old_rlim) == 0); - }); -} TEST(MlockTest, Basic) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(CanMlock())); diff --git a/test/syscalls/linux/select.cc b/test/syscalls/linux/select.cc index 6b6fa9217..41e6043cc 100644 --- a/test/syscalls/linux/select.cc +++ b/test/syscalls/linux/select.cc @@ -12,14 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include -#include +#include +#include #include +#include +#include +#include +#include #include "gtest/gtest.h" #include "gtest/gtest.h" #include "absl/time/time.h" #include "test/syscalls/linux/base_poll_test.h" +#include "test/util/file_descriptor.h" +#include "test/util/multiprocess_util.h" +#include "test/util/posix_error.h" +#include "test/util/rlimit_util.h" +#include "test/util/temp_path.h" #include "test/util/test_util.h" namespace gvisor { @@ -57,15 +66,27 @@ TEST_F(SelectTest, NegativeNfds) { } TEST_F(SelectTest, ClosedFds) { - fd_set read_set; - FD_ZERO(&read_set); - int fd; - ASSERT_THAT(fd = dup(1), SyscallSucceeds()); - ASSERT_THAT(close(fd), SyscallSucceeds()); - FD_SET(fd, &read_set); - struct timeval timeout = absl::ToTimeval(absl::Milliseconds(10)); - EXPECT_THAT(select(fd + 1, &read_set, nullptr, nullptr, &timeout), - SyscallFailsWithErrno(EBADF)); + auto temp_file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(temp_file.path(), O_RDONLY)); + + // We can't rely on a file descriptor being closed in a multi threaded + // application so fork to get a clean process. + EXPECT_THAT(InForkedProcess([&] { + int fd_num = fd.get(); + fd.reset(); + + fd_set read_set; + FD_ZERO(&read_set); + FD_SET(fd_num, &read_set); + + struct timeval timeout = + absl::ToTimeval(absl::Milliseconds(10)); + TEST_PCHECK(select(fd_num + 1, &read_set, nullptr, nullptr, + &timeout) != 0); + TEST_PCHECK(errno == EBADF); + }), + IsPosixErrorOkAndHolds(0)); } TEST_F(SelectTest, ZeroTimeout) { @@ -123,6 +144,25 @@ TEST_F(SelectTest, IgnoreBitsAboveNfds) { SyscallSucceedsWithValue(0)); } +// This test illustrates Linux's behavior of 'select' calls passing after +// setrlimit RLIMIT_NOFILE is called. In particular, versions of sshd rely on +// this behavior. +TEST_F(SelectTest, SetrlimitCallNOFILE) { + fd_set read_set; + FD_ZERO(&read_set); + timeval timeout = {}; + const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE( + Open(NewTempAbsPath(), O_RDONLY | O_CREAT, S_IRUSR)); + + Cleanup reset_rlimit = + ASSERT_NO_ERRNO_AND_VALUE(ScopedSetSoftRlimit(RLIMIT_NOFILE, 0)); + + FD_SET(fd.get(), &read_set); + // this call with zero timeout should return immediately + EXPECT_THAT(select(fd.get() + 1, &read_set, nullptr, nullptr, &timeout), + SyscallSucceeds()); +} + } // namespace } // namespace testing } // namespace gvisor -- cgit v1.2.3