Merge pull request #3125 from sigiesec/improve-socket-inheritance

Improve socket inheritance handling

src/ip.cpp

@@ -80,28 +80,22 @@ zmq::fd_t zmq::open_socket (int domain_, int type_, int protocol_)
     type_ |= SOCK_CLOEXEC;
 #endif
 
-    fd_t s = socket (domain_, type_, protocol_);
-#ifdef ZMQ_HAVE_WINDOWS
-    if (s == INVALID_SOCKET)
-        return INVALID_SOCKET;
+#if defined ZMQ_HAVE_WINDOWS && defined WSA_FLAG_NO_HANDLE_INHERIT
+    // if supported, create socket with WSA_FLAG_NO_HANDLE_INHERIT, such that
+    // the race condition in making it non-inheritable later is avoided
+    const fd_t s = WSASocket (domain_, type_, protocol_, NULL, 0,
+                              WSA_FLAG_NO_HANDLE_INHERIT);
 #else
-    if (s == -1)
-        return -1;
+    const fd_t s = socket (domain_, type_, protocol_);
 #endif
-
-        //  If there's no SOCK_CLOEXEC, let's try the second best option. Note that
-        //  race condition can cause socket not to be closed (if fork happens
-        //  between socket creation and this point).
-#if !defined ZMQ_HAVE_SOCK_CLOEXEC && defined FD_CLOEXEC
-    rc = fcntl (s, F_SETFD, FD_CLOEXEC);
-    errno_assert (rc != -1);
+    if (s == retired_fd) {
+#ifdef ZMQ_HAVE_WINDOWS
+        errno = wsa_error_to_errno (WSAGetLastError ());
 #endif
+        return retired_fd;
+    }
 
-    //  On Windows, preventing sockets to be inherited by child processes.
-#if defined ZMQ_HAVE_WINDOWS && defined HANDLE_FLAG_INHERIT
-    BOOL brc = SetHandleInformation ((HANDLE) s, HANDLE_FLAG_INHERIT, 0);
-    win_assert (brc);
-#endif
+    make_socket_noninheritable (s);
 
     //  Socket is not yet connected so EINVAL is not a valid networking error
     rc = zmq::set_nosigpipe (s);
@@ -530,11 +524,7 @@ int zmq::make_fdpair (fd_t *r_, fd_t *w_)
     }
 
     if (*r_ != INVALID_SOCKET) {
-#if !defined _WIN32_WCE && !defined ZMQ_HAVE_WINDOWS_UWP
-        //  On Windows, preventing sockets to be inherited by child processes.
-        BOOL brc = SetHandleInformation ((HANDLE) *r_, HANDLE_FLAG_INHERIT, 0);
-        win_assert (brc);
-#endif
+        make_socket_noninheritable (*r_);
         return 0;
     } else {
         //  Cleanup writer if connection failed
@@ -659,18 +649,32 @@ int zmq::make_fdpair (fd_t *r_, fd_t *w_)
         *w_ = *r_ = -1;
         return -1;
     } else {
-    //  If there's no SOCK_CLOEXEC, let's try the second best option. Note that
-    //  race condition can cause socket not to be closed (if fork happens
-    //  between socket creation and this point).
-#if !defined ZMQ_HAVE_SOCK_CLOEXEC && defined FD_CLOEXEC
-        rc = fcntl (sv[0], F_SETFD, FD_CLOEXEC);
-        errno_assert (rc != -1);
-        rc = fcntl (sv[1], F_SETFD, FD_CLOEXEC);
-        errno_assert (rc != -1);
-#endif
+        make_socket_noninheritable (sv[0]);
+        make_socket_noninheritable (sv[1]);
+
         *w_ = sv[0];
         *r_ = sv[1];
         return 0;
     }
 #endif
 }
+
+void zmq::make_socket_noninheritable (fd_t sock)
+{
+#if defined ZMQ_HAVE_WINDOWS && !defined _WIN32_WCE                            \
+  && !defined ZMQ_HAVE_WINDOWS_UWP
+    //  On Windows, preventing sockets to be inherited by child processes.
+    const BOOL brc = SetHandleInformation (reinterpret_cast<HANDLE> (sock),
+                                           HANDLE_FLAG_INHERIT, 0);
+    win_assert (brc);
+#endif
+
+#if (!defined ZMQ_HAVE_SOCK_CLOEXEC || !defined HAVE_ACCEPT4)                  \
+  && defined FD_CLOEXEC
+    //  If there 's no SOCK_CLOEXEC, let's try the second best option.
+    //  Race condition can cause socket not to be closed (if fork happens
+    //  between accept and this point).
+    const int rc = fcntl (sock, F_SETFD, FD_CLOEXEC);
+    errno_assert (rc != -1);
+#endif
+}

src/ip.hpp

@@ -67,6 +67,10 @@ void shutdown_network ();
 // Creates a pair of sockets (using signaler_port on OS using TCP sockets).
 // Returns -1 if we could not make the socket pair successfully
 int make_fdpair (fd_t *r_, fd_t *w_);
+
+// Makes a socket non-inheritable to child processes.
+// Asserts on any failure.
+void make_socket_noninheritable (fd_t sock);
 }
 
 #endif

src/ipc_listener.cpp

@@ -403,13 +403,7 @@ zmq::fd_t zmq::ipc_listener_t::accept ()
         return retired_fd;
     }
 
-#if (!defined ZMQ_HAVE_SOCK_CLOEXEC || !defined HAVE_ACCEPT4)                  \
-  && defined FD_CLOEXEC
-    //  Race condition can cause socket not to be closed (if fork happens
-    //  between accept and this point).
-    int rc = fcntl (sock, F_SETFD, FD_CLOEXEC);
-    errno_assert (rc != -1);
-#endif
+    make_socket_noninheritable (sock);
 
     // IPC accept() filters
 #if defined ZMQ_HAVE_SO_PEERCRED || defined ZMQ_HAVE_LOCAL_PEERCRED

src/socks_connecter.cpp

@@ -309,13 +309,8 @@ int zmq::socks_connecter_t::connect_to_proxy ()
 
     //  Create the socket.
     s = open_socket (tcp_addr->family (), SOCK_STREAM, IPPROTO_TCP);
-#ifdef ZMQ_HAVE_WINDOWS
-    if (s == INVALID_SOCKET)
-        return -1;
-#else
-    if (s == -1)
+    if (s == retired_fd)
         return -1;
-#endif
 
     //  On some systems, IPv4 mapping in IPv6 sockets is disabled by default.
     //  Switch it on in such cases.

src/tcp_connecter.cpp

@@ -283,15 +283,9 @@ int zmq::tcp_connecter_t::open ()
         s = open_socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
     }
 
-#ifdef ZMQ_HAVE_WINDOWS
-    if (s == INVALID_SOCKET) {
-        errno = wsa_error_to_errno (WSAGetLastError ());
+    if (s == retired_fd) {
         return -1;
     }
-#else
-    if (s == -1)
-        return -1;
-#endif
 
     //  On some systems, IPv4 mapping in IPv6 sockets is disabled by default.
     //  Switch it on in such cases.

src/tcp_listener.cpp

@@ -195,20 +195,10 @@ int zmq::tcp_listener_t::set_address (const char *addr_)
         s = open_socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
     }
 
-#ifdef ZMQ_HAVE_WINDOWS
-    if (s == INVALID_SOCKET) {
-        errno = wsa_error_to_errno (WSAGetLastError ());
+    if (s == retired_fd) {
         return -1;
     }
-#if !defined _WIN32_WCE && !defined ZMQ_HAVE_WINDOWS_UWP
-    //  On Windows, preventing sockets to be inherited by child processes.
-    BOOL brc = SetHandleInformation ((HANDLE) s, HANDLE_FLAG_INHERIT, 0);
-    win_assert (brc);
-#endif
-#else
-    if (s == -1)
-        return -1;
-#endif
+    make_socket_noninheritable (s);
 
     //  On some systems, IPv4 mapping in IPv6 sockets is disabled by default.
     //  Switch it on in such cases.
@@ -306,35 +296,21 @@ zmq::fd_t zmq::tcp_listener_t::accept ()
       ::accept (s, reinterpret_cast<struct sockaddr *> (&ss), &ss_len);
 #endif
 
+    if (sock == retired_fd) {
 #ifdef ZMQ_HAVE_WINDOWS
-    if (sock == INVALID_SOCKET) {
         const int last_error = WSAGetLastError ();
         wsa_assert (last_error == WSAEWOULDBLOCK || last_error == WSAECONNRESET
                     || last_error == WSAEMFILE || last_error == WSAENOBUFS);
-        return retired_fd;
-    }
-#if !defined _WIN32_WCE && !defined ZMQ_HAVE_WINDOWS_UWP
-    //  On Windows, preventing sockets to be inherited by child processes.
-    BOOL brc = SetHandleInformation ((HANDLE) sock, HANDLE_FLAG_INHERIT, 0);
-    win_assert (brc);
-#endif
 #else
-    if (sock == -1) {
         errno_assert (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR
                       || errno == ECONNABORTED || errno == EPROTO
                       || errno == ENOBUFS || errno == ENOMEM || errno == EMFILE
                       || errno == ENFILE);
+#endif
         return retired_fd;
     }
-#endif
 
-#if (!defined ZMQ_HAVE_SOCK_CLOEXEC || !defined HAVE_ACCEPT4)                  \
-  && defined FD_CLOEXEC
-    //  Race condition can cause socket not to be closed (if fork happens
-    //  between accept and this point).
-    int rc = fcntl (sock, F_SETFD, FD_CLOEXEC);
-    errno_assert (rc != -1);
-#endif
+    make_socket_noninheritable (sock);
 
     if (!options.tcp_accept_filters.empty ()) {
         bool matched = false;