Validate the open file limit when creating a socket

Fixes #16288

Signed-off-by: David Goulet <dgoulet@ev0ke.net>

changes/bug16288

@@ -0,0 +1,6 @@
+  o Major bugfixes (open file limit):
+    - The max open file limit wasn't checked before calling
+      tor_accept_socket_nonblocking() which made tor go beyond the open
+      file limit set previously. With this fix, before opening a new socket,
+      tor validates the open file limit just before and if the max has been
+      reached, return EMFILE.; Fixes #16288; bugfix on tor-0.1.1.1-alpha~74.

src/common/compat.c

@@ -131,6 +131,11 @@
 #include "strlcat.c"
 #endif
 
+/* When set_max_file_descriptors() is called, update this with the max file
+ * descriptor value so we can use it to check the limit when opening a new
+ * socket. Default value is what Debian sets as the default hard limit. */
+static int max_sockets = 1024;
+
 /** As open(path, flags, mode), but return an fd with the close-on-exec mode
  * set. */
 int
@@ -1187,6 +1192,18 @@ tor_open_socket_with_extensions(int domain, int type, int protocol,
                                 int cloexec, int nonblock)
 {
   tor_socket_t s;
+
+  /* We are about to create a new file descriptor so make sure we have
+   * enough of them. */
+  if (get_n_open_sockets() >= max_sockets - 1) {
+#ifdef _WIN32
+    WSASetLastError(WSAEMFILE);
+#else
+    errno = EMFILE;
+#endif
+    return TOR_INVALID_SOCKET;
+  }
+
 #if defined(SOCK_CLOEXEC) && defined(SOCK_NONBLOCK)
   int ext_flags = (cloexec ? SOCK_CLOEXEC : 0) |
                   (nonblock ? SOCK_NONBLOCK : 0);
@@ -1258,6 +1275,18 @@ tor_accept_socket_with_extensions(tor_socket_t sockfd, struct sockaddr *addr,
                                  socklen_t *len, int cloexec, int nonblock)
 {
   tor_socket_t s;
+
+  /* We are about to create a new file descriptor so make sure we have
+   * enough of them. */
+  if (get_n_open_sockets() >= max_sockets - 1) {
+#ifdef _WIN32
+    WSASetLastError(WSAEMFILE);
+#else
+    errno = EMFILE;
+#endif
+    return TOR_INVALID_SOCKET;
+  }
+
 #if defined(HAVE_ACCEPT4) && defined(SOCK_CLOEXEC) && defined(SOCK_NONBLOCK)
   int ext_flags = (cloexec ? SOCK_CLOEXEC : 0) |
                   (nonblock ? SOCK_NONBLOCK : 0);
@@ -1553,6 +1582,12 @@ tor_ersatz_socketpair(int family, int type, int protocol, tor_socket_t fd[2])
 int
 set_max_file_descriptors(rlim_t limit, int *max_out)
 {
+  if (limit < ULIMIT_BUFFER) {
+    log_warn(LD_CONFIG,
+             "ConnLimit must be at least %d. Failing.", ULIMIT_BUFFER);
+    return -1;
+  }
+
   /* Define some maximum connections values for systems where we cannot
    * automatically determine a limit. Re Cygwin, see
    * http://archives.seul.org/or/talk/Aug-2006/msg00210.html
@@ -1592,7 +1627,7 @@ set_max_file_descriptors(rlim_t limit, int *max_out)
     limit = rlim.rlim_max;
     if (limit > INT_MAX)
       limit = INT_MAX;
-    *max_out = (int)limit - ULIMIT_BUFFER;
+    *max_out = max_sockets = (int)limit - ULIMIT_BUFFER;
     return 0;
   }
   if (rlim.rlim_max < limit) {
@@ -1606,6 +1641,9 @@ set_max_file_descriptors(rlim_t limit, int *max_out)
     log_info(LD_NET,"Raising max file descriptors from %lu to %lu.",
              (unsigned long)rlim.rlim_cur, (unsigned long)rlim.rlim_max);
   }
+  /* Set the current limit value so if the attempt to set the limit to the
+   * max fails at least we'll have a valid value of maximum sockets. */
+  max_sockets = rlim.rlim_cur - ULIMIT_BUFFER;
   rlim.rlim_cur = rlim.rlim_max;
 
   if (setrlimit(RLIMIT_NOFILE, &rlim) != 0) {
@@ -1639,15 +1677,10 @@ set_max_file_descriptors(rlim_t limit, int *max_out)
   limit = rlim.rlim_cur;
 #endif /* HAVE_GETRLIMIT */
 
-  if (limit < ULIMIT_BUFFER) {
-    log_warn(LD_CONFIG,
-             "ConnLimit must be at least %d. Failing.", ULIMIT_BUFFER);
-    return -1;
-  }
   if (limit > INT_MAX)
     limit = INT_MAX;
   tor_assert(max_out);
-  *max_out = (int)limit - ULIMIT_BUFFER;
+  *max_out = max_sockets = (int)limit - ULIMIT_BUFFER;
   return 0;
 }
 

src/common/compat.h

@@ -580,7 +580,7 @@ int network_init(void);
 #define ERRNO_IS_ACCEPT_EAGAIN(e)    ERRNO_IS_EAGAIN(e)
 /** Return true if e is EMFILE or another error indicating that a call to
  * accept() has failed because we're out of fds or something. */
-#define ERRNO_IS_ACCEPT_RESOURCE_LIMIT(e) \
+#define ERRNO_IS_RESOURCE_LIMIT(e) \
   ((e) == WSAEMFILE || (e) == WSAENOBUFS)
 /** Return true if e is EADDRINUSE or the local equivalent. */
 #define ERRNO_IS_EADDRINUSE(e)      ((e) == WSAEADDRINUSE)
@@ -598,7 +598,7 @@ const char *tor_socket_strerror(int e);
 #define ERRNO_IS_CONN_EINPROGRESS(e) ((e) == EINPROGRESS || 0)
 #define ERRNO_IS_ACCEPT_EAGAIN(e) \
   (ERRNO_IS_EAGAIN(e) || (e) == ECONNABORTED)
-#define ERRNO_IS_ACCEPT_RESOURCE_LIMIT(e) \
+#define ERRNO_IS_RESOURCE_LIMIT(e) \
   ((e) == EMFILE || (e) == ENFILE || (e) == ENOBUFS || (e) == ENOMEM)
 #define ERRNO_IS_EADDRINUSE(e)       (((e) == EADDRINUSE) || 0)
 #define tor_socket_errno(sock)       (errno)

src/or/connection.c

@@ -1093,11 +1093,6 @@ connection_listener_new(const struct sockaddr *listensockaddr,
   static int global_next_session_group = SESSION_GROUP_FIRST_AUTO;
   tor_addr_t addr;
 
-  if (get_n_open_sockets() >= options->ConnLimit_-1) {
-    warn_too_many_conns();
-    return NULL;
-  }
-
   if (listensockaddr->sa_family == AF_INET ||
       listensockaddr->sa_family == AF_INET6) {
     int is_stream = (type != CONN_TYPE_AP_DNS_LISTENER);
@@ -1113,8 +1108,13 @@ connection_listener_new(const struct sockaddr *listensockaddr,
       is_stream ? SOCK_STREAM : SOCK_DGRAM,
       is_stream ? IPPROTO_TCP: IPPROTO_UDP);
     if (!SOCKET_OK(s)) {
-      log_warn(LD_NET, "Socket creation failed: %s",
+      int e = tor_socket_errno(s);
-               tor_socket_strerror(tor_socket_errno(-1)));
+      if (ERRNO_IS_RESOURCE_LIMIT(e)) {
+        warn_too_many_conns();
+      } else {
+        log_warn(LD_NET, "Socket creation failed: %s",
+                 tor_socket_strerror(e));
+      }
       goto err;
     }
 
@@ -1222,7 +1222,12 @@ connection_listener_new(const struct sockaddr *listensockaddr,
 
     s = tor_open_socket_nonblocking(AF_UNIX, SOCK_STREAM, 0);
     if (! SOCKET_OK(s)) {
-      log_warn(LD_NET,"Socket creation failed: %s.", strerror(errno));
+      int e = tor_socket_errno(s);
+      if (ERRNO_IS_RESOURCE_LIMIT(e)) {
+        warn_too_many_conns();
+      } else {
+        log_warn(LD_NET,"Socket creation failed: %s.", strerror(e));
+      }
       goto err;
     }
 
@@ -1430,7 +1435,7 @@ connection_handle_listener_read(connection_t *conn, int new_type)
     int e = tor_socket_errno(conn->s);
     if (ERRNO_IS_ACCEPT_EAGAIN(e)) {
       return 0; /* he hung up before we could accept(). that's fine. */
-    } else if (ERRNO_IS_ACCEPT_RESOURCE_LIMIT(e)) {
+    } else if (ERRNO_IS_RESOURCE_LIMIT(e)) {
       warn_too_many_conns();
       return 0;
     }
@@ -1624,12 +1629,6 @@ connection_connect_sockaddr(connection_t *conn,
   tor_assert(sa);
   tor_assert(socket_error);
 
-  if (get_n_open_sockets() >= get_options()->ConnLimit_-1) {
-    warn_too_many_conns();
-    *socket_error = SOCK_ERRNO(ENOBUFS);
-    return -1;
-  }
-
   if (get_options()->DisableNetwork) {
     /* We should never even try to connect anyplace if DisableNetwork is set.
      * Warn if we do, and refuse to make the connection. */
@@ -1647,9 +1646,13 @@ connection_connect_sockaddr(connection_t *conn,
 
   s = tor_open_socket_nonblocking(protocol_family, SOCK_STREAM, proto);
   if (! SOCKET_OK(s)) {
-    *socket_error = tor_socket_errno(-1);
+    *socket_error = tor_socket_errno(s);
-    log_warn(LD_NET,"Error creating network socket: %s",
+    if (ERRNO_IS_RESOURCE_LIMIT(*socket_error)) {
-             tor_socket_strerror(*socket_error));
+      warn_too_many_conns();
+    } else {
+      log_warn(LD_NET,"Error creating network socket: %s",
+               tor_socket_strerror(*socket_error));
+    }
     return -1;
   }