Fix bug with TCP bind w/ SO_REUSEADDR.

In gVisor today its possible that when trying to bind a TCP socket
w/ SO_REUSEADDR specified and requesting the kernel pick a port by
setting port to zero can result in a previously bound port being
returned. This behaviour is incorrect as the user is clearly requesting
a free port. The behaviour is fine when the user explicity specifies
a port.

This change now checks if the user specified a port when making a port
reservation for a TCP port and only returns unbound ports even if
SO_REUSEADDR was specified.

Fixes #6209

PiperOrigin-RevId: 382607638

4 files changed, 113 insertions, 52 deletions

diff --git a/test/syscalls/linux/socket_generic_stress.cc b/test/syscalls/linux/socket_generic_stress.cc
index c35aa2183..778c32a8e 100644
--- a/test/syscalls/linux/socket_generic_stress.cc
+++ b/test/syscalls/linux/socket_generic_stress.cc
@@ -37,49 +37,11 @@
 namespace gvisor {
 namespace testing {
 
-constexpr char kRangeFile[] = "/proc/sys/net/ipv4/ip_local_port_range";
-
-PosixErrorOr<int> NumPorts() {
-  int min = 0;
-  int max = 1 << 16;
-
-  // Read the ephemeral range from /proc.
-  ASSIGN_OR_RETURN_ERRNO(std::string rangefile, GetContents(kRangeFile));
-  const std::string err_msg =
-      absl::StrFormat("%s has invalid content: %s", kRangeFile, rangefile);
-  if (rangefile.back() != '\n') {
-    return PosixError(EINVAL, err_msg);
-  }
-  rangefile.pop_back();
-  std::vector<std::string> range =
-      absl::StrSplit(rangefile, absl::ByAnyChar("\t "));
-  if (range.size() < 2 || !absl::SimpleAtoi(range.front(), &min) ||
-      !absl::SimpleAtoi(range.back(), &max)) {
-    return PosixError(EINVAL, err_msg);
-  }
-
-  // If we can open as writable, limit the range.
-  if (!access(kRangeFile, W_OK)) {
-    ASSIGN_OR_RETURN_ERRNO(FileDescriptor fd,
-                           Open(kRangeFile, O_WRONLY | O_TRUNC, 0));
-    max = min + 50;
-    const std::string small_range = absl::StrFormat("%d %d", min, max);
-    int n = write(fd.get(), small_range.c_str(), small_range.size());
-    if (n < 0) {
-      return PosixError(
-          errno,
-          absl::StrFormat("write(%d [%s], \"%s\", %d)", fd.get(), kRangeFile,
-                          small_range.c_str(), small_range.size()));
-    }
-  }
-  return max - min;
-}
-
 // Test fixture for tests that apply to pairs of connected sockets.
 using ConnectStressTest = SocketPairTest;
 
 TEST_P(ConnectStressTest, Reset) {
-  const int nports = ASSERT_NO_ERRNO_AND_VALUE(NumPorts());
+  const int nports = ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
   for (int i = 0; i < nports * 2; i++) {
     const std::unique_ptr<SocketPair> sockets =
         ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
@@ -103,7 +65,7 @@ TEST_P(ConnectStressTest, Reset) {
 // Tests that opening too many connections -- without closing them -- does lead
 // to port exhaustion.
 TEST_P(ConnectStressTest, TooManyOpen) {
-  const int nports = ASSERT_NO_ERRNO_AND_VALUE(NumPorts());
+  const int nports = ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
   int err_num = 0;
   std::vector<std::unique_ptr<SocketPair>> sockets =
       std::vector<std::unique_ptr<SocketPair>>(nports);
@@ -164,7 +126,7 @@ class PersistentListenerConnectStressTest : public SocketPairTest {
 };
 
 TEST_P(PersistentListenerConnectStressTest, ShutdownCloseFirst) {
-  const int nports = ASSERT_NO_ERRNO_AND_VALUE(NumPorts());
+  const int nports = ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
   for (int i = 0; i < nports * 2; i++) {
     std::unique_ptr<SocketPair> sockets =
         ASSERT_NO_ERRNO_AND_VALUE(NewSocketSleep());
@@ -185,7 +147,7 @@ TEST_P(PersistentListenerConnectStressTest, ShutdownCloseFirst) {
 }
 
 TEST_P(PersistentListenerConnectStressTest, ShutdownCloseSecond) {
-  const int nports = ASSERT_NO_ERRNO_AND_VALUE(NumPorts());
+  const int nports = ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
   for (int i = 0; i < nports * 2; i++) {
     const std::unique_ptr<SocketPair> sockets =
         ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair());
@@ -206,7 +168,7 @@ TEST_P(PersistentListenerConnectStressTest, ShutdownCloseSecond) {
 }
 
 TEST_P(PersistentListenerConnectStressTest, Close) {
-  const int nports = ASSERT_NO_ERRNO_AND_VALUE(NumPorts());
+  const int nports = ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
   for (int i = 0; i < nports * 2; i++) {
     std::unique_ptr<SocketPair> sockets =
         ASSERT_NO_ERRNO_AND_VALUE(NewSocketSleep());
diff --git a/test/syscalls/linux/socket_inet_loopback_nogotsan.cc b/test/syscalls/linux/socket_inet_loopback_nogotsan.cc
index b131213d4..cc2773af1 100644
--- a/test/syscalls/linux/socket_inet_loopback_nogotsan.cc
+++ b/test/syscalls/linux/socket_inet_loopback_nogotsan.cc
@@ -104,16 +104,25 @@ INSTANTIATE_TEST_SUITE_P(All, SocketInetLoopbackTest,
 using SocketMultiProtocolInetLoopbackTest =
     ::testing::TestWithParam<ProtocolTestParam>;
 
-TEST_P(SocketMultiProtocolInetLoopbackTest, BindAvoidsListeningPortsReuseAddr) {
+TEST_P(SocketMultiProtocolInetLoopbackTest,
+       TCPBindAvoidsOtherBoundPortsReuseAddr) {
   ProtocolTestParam const& param = GetParam();
-  // UDP sockets are allowed to bind/listen on the port w/ SO_REUSEADDR, for TCP
-  // this is only permitted if there is no other listening socket.
+  // UDP sockets are allowed to bind/listen on an already bound port w/
+  // SO_REUSEADDR even when requesting a port from the kernel. In case of TCP
+  // rebinding is only permitted when SO_REUSEADDR is set and an explicit port
+  // is specified. When a zero port is specified to the bind() call then an
+  // already bound port will not be picked.
   SKIP_IF(param.type != SOCK_STREAM);
 
   DisableSave ds;  // Too many syscalls.
 
   // A map of port to file descriptor binding the port.
-  std::map<uint16_t, FileDescriptor> listen_sockets;
+  std::map<uint16_t, FileDescriptor> bound_sockets;
+
+  // Reduce number of ephemeral ports if permitted to reduce running time of
+  // the test.
+  [[maybe_unused]] const int nports =
+      ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
 
   // Exhaust all ephemeral ports.
   while (true) {
@@ -139,12 +148,59 @@ TEST_P(SocketMultiProtocolInetLoopbackTest, BindAvoidsListeningPortsReuseAddr) {
         SyscallSucceeds());
     uint16_t port = reinterpret_cast<sockaddr_in*>(&bound_addr)->sin_port;
 
-    // Newly bound port should not already be in use by a listening socket.
-    ASSERT_EQ(listen_sockets.find(port), listen_sockets.end());
-    auto fd = bound_fd.get();
-    listen_sockets.insert(std::make_pair(port, std::move(bound_fd)));
-    ASSERT_THAT(listen(fd, SOMAXCONN), SyscallSucceeds());
+    auto [iter, inserted] = bound_sockets.emplace(port, std::move(bound_fd));
+    ASSERT_TRUE(inserted);
+  }
+}
+
+TEST_P(SocketMultiProtocolInetLoopbackTest,
+       UDPBindMayBindOtherBoundPortsReuseAddr) {
+  ProtocolTestParam const& param = GetParam();
+  // UDP sockets are allowed to bind/listen on an already bound port w/
+  // SO_REUSEADDR even when requesting a port from the kernel.
+  SKIP_IF(param.type != SOCK_DGRAM);
+
+  DisableSave ds;  // Too many syscalls.
+
+  // A map of port to file descriptor binding the port.
+  std::map<uint16_t, FileDescriptor> bound_sockets;
+
+  // Reduce number of ephemeral ports if permitted to reduce running time of
+  // the test.
+  [[maybe_unused]] const int nports =
+      ASSERT_NO_ERRNO_AND_VALUE(MaybeLimitEphemeralPorts());
+
+  // Exhaust all ephemeral ports.
+  bool duplicate_binding = false;
+  while (true) {
+    // Bind the v4 loopback on a v4 socket.
+    TestAddress const& test_addr = V4Loopback();
+    sockaddr_storage bound_addr = test_addr.addr;
+    FileDescriptor bound_fd =
+        ASSERT_NO_ERRNO_AND_VALUE(Socket(test_addr.family(), param.type, 0));
+
+    ASSERT_THAT(setsockopt(bound_fd.get(), SOL_SOCKET, SO_REUSEADDR,
+                           &kSockOptOn, sizeof(kSockOptOn)),
+                SyscallSucceeds());
+
+    ASSERT_THAT(
+        bind(bound_fd.get(), AsSockAddr(&bound_addr), test_addr.addr_len),
+        SyscallSucceeds());
+
+    // Get the port that we bound.
+    socklen_t bound_addr_len = test_addr.addr_len;
+    ASSERT_THAT(
+        getsockname(bound_fd.get(), AsSockAddr(&bound_addr), &bound_addr_len),
+        SyscallSucceeds());
+    uint16_t port = reinterpret_cast<sockaddr_in*>(&bound_addr)->sin_port;
+
+    auto [iter, inserted] = bound_sockets.emplace(port, std::move(bound_fd));
+    if (!inserted) {
+      duplicate_binding = true;
+      break;
+    }
   }
+  ASSERT_TRUE(duplicate_binding);
 }
 
 INSTANTIATE_TEST_SUITE_P(AllFamilies, SocketMultiProtocolInetLoopbackTest,
diff --git a/test/syscalls/linux/socket_test_util.cc b/test/syscalls/linux/socket_test_util.cc
index 5e36472b4..1afb1ab50 100644
--- a/test/syscalls/linux/socket_test_util.cc
+++ b/test/syscalls/linux/socket_test_util.cc
@@ -24,6 +24,7 @@
 #include "gtest/gtest.h"
 #include "absl/memory/memory.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_split.h"
 #include "absl/time/clock.h"
 #include "absl/types/optional.h"
 #include "test/util/file_descriptor.h"
@@ -1067,5 +1068,43 @@ void SetupTimeWaitClose(const TestAddress* listener,
   absl::SleepFor(absl::Seconds(1));
 }
 
+constexpr char kRangeFile[] = "/proc/sys/net/ipv4/ip_local_port_range";
+
+PosixErrorOr<int> MaybeLimitEphemeralPorts() {
+  int min = 0;
+  int max = 1 << 16;
+
+  // Read the ephemeral range from /proc.
+  ASSIGN_OR_RETURN_ERRNO(std::string rangefile, GetContents(kRangeFile));
+  const std::string err_msg =
+      absl::StrFormat("%s has invalid content: %s", kRangeFile, rangefile);
+  if (rangefile.back() != '\n') {
+    return PosixError(EINVAL, err_msg);
+  }
+  rangefile.pop_back();
+  std::vector<std::string> range =
+      absl::StrSplit(rangefile, absl::ByAnyChar("\t "));
+  if (range.size() < 2 || !absl::SimpleAtoi(range.front(), &min) ||
+      !absl::SimpleAtoi(range.back(), &max)) {
+    return PosixError(EINVAL, err_msg);
+  }
+
+  // If we can open as writable, limit the range.
+  if (!access(kRangeFile, W_OK)) {
+    ASSIGN_OR_RETURN_ERRNO(FileDescriptor fd,
+                           Open(kRangeFile, O_WRONLY | O_TRUNC, 0));
+    max = min + 50;
+    const std::string small_range = absl::StrFormat("%d %d", min, max);
+    int n = write(fd.get(), small_range.c_str(), small_range.size());
+    if (n < 0) {
+      return PosixError(
+          errno,
+          absl::StrFormat("write(%d [%s], \"%s\", %d)", fd.get(), kRangeFile,
+                          small_range.c_str(), small_range.size()));
+    }
+  }
+  return max - min;
+}
+
 }  // namespace testing
 }  // namespace gvisor
diff --git a/test/syscalls/linux/socket_test_util.h b/test/syscalls/linux/socket_test_util.h
index df4c26f26..0e2be63cc 100644
--- a/test/syscalls/linux/socket_test_util.h
+++ b/test/syscalls/linux/socket_test_util.h
@@ -576,6 +576,10 @@ void SetupTimeWaitClose(const TestAddress* listener,
                         bool accept_close, sockaddr_storage* listen_addr,
                         sockaddr_storage* conn_bound_addr);
 
+// MaybeLimitEphemeralPorts attempts to reduce the number of ephemeral ports and
+// returns the number of ephemeral ports.
+PosixErrorOr<int> MaybeLimitEphemeralPorts();
+
 namespace internal {
 PosixErrorOr<int> TryPortAvailable(int port, AddressFamily family,
                                    SocketType type, bool reuse_addr);