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

3 files changed, 60 insertions, 27 deletions

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 <errno.h>
-#include <string.h>
 #include <sys/mman.h>
 #include <sys/resource.h>
 #include <sys/syscall.h>
 #include <unistd.h>
+#include <cerrno>
+#include <cstring>
 
 #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<Cleanup> 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 <limits.h>
-#include <signal.h>
+#include <fcntl.h>
+#include <sys/resource.h>
 #include <sys/select.h>
+#include <sys/time.h>
+#include <climits>
+#include <csignal>
+#include <cstdio>
 
 #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