Bug27795 27782

changes/bug27782

@@ -0,0 +1,4 @@
+  o Minor bugfixes (NSS):
+    - Correctly detect failure to open a dummy TCP socket when
+      stealing ownership of an fd from the NSS layer. Fixes bug 27782;
+      bugfix on 0.3.5.1-alpha.

changes/bug27795

@@ -0,0 +1,5 @@
+  o Major bugfixes (socket accounting):
+    - In our socket accounting code, count a socket as closed even
+      when it is closed indirectly by the TLS layer. Previously, we
+      would count these sockets as still in use, and incorrectly believe that
+      we had run out of sockets. Fixes bug 27795; bugfix on 0.3.5.1-alpha.

src/core/mainloop/connection.c

@@ -646,6 +646,7 @@ connection_free_minimal(connection_t *conn)
       } else {
         /* The tor_tls_free() call below will close the socket; we must tell
          * the code below not to close it a second time. */
+        tor_release_socket_ownership(conn->s);
         conn->s = TOR_INVALID_SOCKET;
       }
       tor_tls_free(or_conn->tls);

src/lib/net/socket.c

@@ -142,41 +142,6 @@ tor_close_socket_simple(tor_socket_t s)
   return r;
 }
 
-/** As tor_close_socket_simple(), but keeps track of the number
- * of open sockets. Returns 0 on success, -1 on failure. */
-MOCK_IMPL(int,
-tor_close_socket,(tor_socket_t s))
-{
-  int r = tor_close_socket_simple(s);
-
-  socket_accounting_lock();
-#ifdef DEBUG_SOCKET_COUNTING
-  if (s > max_socket || ! bitarray_is_set(open_sockets, s)) {
-    log_warn(LD_BUG, "Closing a socket (%d) that wasn't returned by tor_open_"
-             "socket(), or that was already closed or something.", s);
-  } else {
-    tor_assert(open_sockets && s <= max_socket);
-    bitarray_clear(open_sockets, s);
-  }
-#endif /* defined(DEBUG_SOCKET_COUNTING) */
-  if (r == 0) {
-    --n_sockets_open;
-  } else {
-#ifdef _WIN32
-    if (r != WSAENOTSOCK)
-      --n_sockets_open;
-#else
-    if (r != EBADF)
-      --n_sockets_open; // LCOV_EXCL_LINE -- EIO and EINTR too hard to force.
-#endif /* defined(_WIN32) */
-    r = -1;
-  }
-
-  tor_assert_nonfatal(n_sockets_open >= 0);
-  socket_accounting_unlock();
-  return r;
-}
-
 /** @{ */
 #ifdef DEBUG_SOCKET_COUNTING
 /** Helper: if DEBUG_SOCKET_COUNTING is enabled, remember that <b>s</b> is
@@ -201,11 +166,50 @@ mark_socket_open(tor_socket_t s)
   }
   bitarray_set(open_sockets, s);
 }
+static inline void
+mark_socket_closed(tor_socket_t s)
+{
+  if (s > max_socket || ! bitarray_is_set(open_sockets, s)) {
+    log_warn(LD_BUG, "Closing a socket (%d) that wasn't returned by tor_open_"
+             "socket(), or that was already closed or something.", s);
+  } else {
+    tor_assert(open_sockets && s <= max_socket);
+    bitarray_clear(open_sockets, s);
+  }
+}
 #else /* !(defined(DEBUG_SOCKET_COUNTING)) */
 #define mark_socket_open(s) ((void) (s))
+#define mark_socket_closed(s) ((void) (s))
 #endif /* defined(DEBUG_SOCKET_COUNTING) */
 /** @} */
 
+/** As tor_close_socket_simple(), but keeps track of the number
+ * of open sockets. Returns 0 on success, -1 on failure. */
+MOCK_IMPL(int,
+tor_close_socket,(tor_socket_t s))
+{
+  int r = tor_close_socket_simple(s);
+
+  socket_accounting_lock();
+  mark_socket_closed(s);
+  if (r == 0) {
+    --n_sockets_open;
+  } else {
+#ifdef _WIN32
+    if (r != WSAENOTSOCK)
+      --n_sockets_open;
+#else
+    if (r != EBADF)
+      --n_sockets_open; // LCOV_EXCL_LINE -- EIO and EINTR too hard to force.
+#endif /* defined(_WIN32) */
+    r = -1;
+  }
+
+  tor_assert_nonfatal(n_sockets_open >= 0);
+  socket_accounting_unlock();
+  return r;
+}
+
 /** As socket(), but counts the number of open sockets. */
 MOCK_IMPL(tor_socket_t,
 tor_open_socket,(int domain, int type, int protocol))
@@ -307,6 +311,20 @@ tor_take_socket_ownership(tor_socket_t s)
   socket_accounting_unlock();
 }
 
+/**
+ * For socket accounting: declare that we are no longer the owner of the
+ * socket <b>s</b>. This will prevent us from overallocating sockets, and
+ * prevent us from asserting later when we close the socket <b>s</b>.
+ */
+void
+tor_release_socket_ownership(tor_socket_t s)
+{
+  socket_accounting_lock();
+  --n_sockets_open;
+  mark_socket_closed(s);
+  socket_accounting_unlock();
+}
+
 /** As accept(), but counts the number of open sockets. */
 tor_socket_t
 tor_accept_socket(tor_socket_t sockfd, struct sockaddr *addr, socklen_t *len)

src/lib/net/socket.h

@@ -23,6 +23,7 @@ struct sockaddr;
 int tor_close_socket_simple(tor_socket_t s);
 MOCK_DECL(int, tor_close_socket, (tor_socket_t s));
 void tor_take_socket_ownership(tor_socket_t s);
+void tor_release_socket_ownership(tor_socket_t s);
 tor_socket_t tor_open_socket_with_extensions(
                                            int domain, int type, int protocol,
                                            int cloexec, int nonblock);

src/lib/tls/tortls_nss.c

@@ -444,18 +444,22 @@ tor_tls_release_socket(tor_tls_t *tls)
    */
   tor_socket_t sock =
     tor_open_socket_nonblocking(AF_INET, SOCK_STREAM, IPPROTO_TCP);
-  if (!sock) {
+  if (! SOCKET_OK(sock)) {
     log_warn(LD_NET, "Out of sockets when trying to shut down an NSS "
              "connection");
     return;
   }
 
   PRFileDesc *tcp = PR_GetIdentitiesLayer(tls->ssl, PR_NSPR_IO_LAYER);
   if (BUG(! tcp)) {
+    tor_close_socket(sock);
     return;
   }
 
   PR_ChangeFileDescNativeHandle(tcp, sock);
+  /* Tell our socket accounting layer that we don't own this socket any more:
+   * NSS is about to free it for us. */
+  tor_release_socket_ownership(sock);
 }
 
 void