commit 699acd8d54bd685b135d3a8e758d05dd0802b820 Author: David Goulet dgoulet@ev0ke.net Date: Wed Jun 3 13:56:01 2015 -0400

Validate the open file limit when creating a socket

Fixes #16288

Signed-off-by: David Goulet dgoulet@ev0ke.net --- changes/bug16288 | 6 ++++++ src/common/compat.c | 47 ++++++++++++++++++++++++++++++++++++++++------- src/common/compat.h | 4 ++-- src/or/connection.c | 39 +++++++++++++++++++++------------------ 4 files changed, 69 insertions(+), 27 deletions(-)

diff --git a/changes/bug16288 b/changes/bug16288 new file mode 100644 index 0000000..7ddf615 --- /dev/null +++ b/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. diff --git a/src/common/compat.c b/src/common/compat.c index 8da7ef3..ac47321 100644 --- a/src/common/compat.c +++ b/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; }

diff --git a/src/common/compat.h b/src/common/compat.h index 5189b7e..acf5ffd 100644 --- a/src/common/compat.h +++ b/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) diff --git a/src/or/connection.c b/src/or/connection.c index bd17629..7089292 100644 --- a/src/or/connection.c +++ b/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", - tor_socket_strerror(tor_socket_errno(-1))); + 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", + 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); - log_warn(LD_NET,"Error creating network socket: %s", - tor_socket_strerror(*socket_error)); + *socket_error = tor_socket_errno(s); + if (ERRNO_IS_RESOURCE_LIMIT(*socket_error)) { + warn_too_many_conns(); + } else { + log_warn(LD_NET,"Error creating network socket: %s", + tor_socket_strerror(*socket_error)); + } return -1; }