* Unify socket creation and accepting by using utility functions

* Check SO_ERROR on socket immideately after connect/bind to avoid resourses allocation for
  unsuccessful connections
* Avoid descriptors leakage when we create sockets and have errors on them
* Set on all socket descriptors FD_CLOEXEC flag to avoid problems with executing LDA

diff --git a/src/controller.c b/src/controller.c
index b02782caf38cb215c42810a13130db31313edee0..26c773cffb96df21b3dff8830517997f7a212d36 100644 (file)
--- a/src/controller.c
+++ b/src/controller.c
@@ -477,13 +477,11 @@ accept_socket (int fd, short what, void *arg)
        socklen_t addrlen = sizeof(ss);
        int nfd;
 
-       if ((nfd = accept (fd, (struct sockaddr *)&ss, &addrlen)) == -1) {
+       if ((nfd = accept_from_socket (fd, (struct sockaddr *)&ss, &addrlen)) == -1) {
+               msg_warn ("accept_socket: accept failed: %s", strerror (errno));
                return;
        }
-       if (event_make_socket_nonblocking(fd) < 0) {
-               return;
-       }
-       
+
        new_session = g_malloc (sizeof (struct controller_session));
        if (new_session == NULL) {
                msg_err ("accept_socket: cannot allocate memory for task, %s", strerror (errno));
@@ -527,14 +525,14 @@ start_controller (struct rspamd_worker *worker)
        signal_add (&worker->sig_ev, NULL);
 
        if (worker->srv->cfg->control_family == AF_INET) {
-               if ((listen_sock = make_socket (&worker->srv->cfg->control_addr, worker->srv->cfg->control_port)) == -1) {
+               if ((listen_sock = make_tcp_socket (&worker->srv->cfg->control_addr, worker->srv->cfg->control_port, TRUE)) == -1) {
                        msg_err ("start_controller: cannot create tcp listen socket. %s", strerror (errno));
                        exit(-errno);
                }
        }
        else {
                un_addr = (struct sockaddr_un *) alloca (sizeof (struct sockaddr_un));
-               if (!un_addr || (listen_sock = make_unix_socket (worker->srv->cfg->control_host, un_addr)) == -1) {
+               if (!un_addr || (listen_sock = make_unix_socket (worker->srv->cfg->control_host, un_addr, TRUE)) == -1) {
                        msg_err ("start_controller: cannot create unix listen socket. %s", strerror (errno));
                        exit(-errno);
                }

diff --git a/src/lmtp.c b/src/lmtp.c
index aa5c034ff2377f4e763c9d18fe4c97a23bdfe485..276aad0cbebdf8d0a6a546c42b84925ab37950cf 100644 (file)
--- a/src/lmtp.c
+++ b/src/lmtp.c
@@ -209,22 +209,12 @@ accept_socket (int fd, short what, void *arg)
        struct worker_task *new_task;
        struct rspamd_lmtp_proto *lmtp;
        socklen_t addrlen = sizeof(ss);
-       int nfd, on = 1;
-       struct linger linger;
+       int nfd;
 
-       if ((nfd = accept (fd, (struct sockaddr *)&ss, &addrlen)) == -1) {
+       if ((nfd = accept_from_socket (fd, (struct sockaddr *)&ss, &addrlen)) == -1) {
+               msg_warn ("accept_socket: accept failed: %s", strerror (errno));
                return;
        }
-       if (event_make_socket_nonblocking(fd) < 0) {
-               return;
-       }
-
-       /* Socket options */
-       setsockopt (nfd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on));
-       setsockopt (nfd, SOL_SOCKET, SO_REUSEADDR, (void *)&on, sizeof(on));
-       linger.l_onoff = 1;
-       linger.l_linger = 2;
-       setsockopt (nfd, SOL_SOCKET, SO_LINGER, (void *)&linger, sizeof(linger));
 
        lmtp = g_malloc (sizeof (struct rspamd_lmtp_proto));
        new_task = g_malloc (sizeof (struct worker_task));
@@ -276,14 +266,14 @@ start_lmtp_worker (struct rspamd_worker *worker)
        
        /* Create listen socket */
        if (worker->srv->cfg->lmtp_family == AF_INET) {
-               if ((listen_sock = make_socket (&worker->srv->cfg->lmtp_addr, worker->srv->cfg->lmtp_port)) == -1) {
+               if ((listen_sock = make_tcp_socket (&worker->srv->cfg->lmtp_addr, worker->srv->cfg->lmtp_port, TRUE)) == -1) {
                        msg_err ("start_lmtp: cannot create tcp listen socket. %s", strerror (errno));
                        exit(-errno);
                }
        }
        else {
                un_addr = (struct sockaddr_un *) alloca (sizeof (struct sockaddr_un));
-               if (!un_addr || (listen_sock = make_unix_socket (worker->srv->cfg->lmtp_host, un_addr)) == -1) {
+               if (!un_addr || (listen_sock = make_unix_socket (worker->srv->cfg->lmtp_host, un_addr, TRUE)) == -1) {
                        msg_err ("start_lmtp: cannot create unix listen socket. %s", strerror (errno));
                        exit(-errno);
                }

diff --git a/src/lmtp_proto.c b/src/lmtp_proto.c
index 42bf3560a8979d4da8f5dd1d6593f254deed23ba..ab20e41a8b72b274f400b17afdde04257159fc09 100644 (file)
--- a/src/lmtp_proto.c
+++ b/src/lmtp_proto.c
@@ -416,31 +416,20 @@ mta_err_socket (GError *err, void *arg)
 static int
 lmtp_deliver_mta (struct worker_task *task)
 {
-       int sock, on = 1;
-       struct linger linger;
+       int sock;
        struct sockaddr_un *un;
        struct mta_callback_data *cd;
        
        if (task->cfg->deliver_family == AF_UNIX) {
                un = alloca (sizeof (struct sockaddr_un));
-               sock = make_unix_socket (task->cfg->deliver_host, un);
-               if (event_make_socket_nonblocking (sock) < 0) {
-                       return -1;
-               }
+               sock = make_unix_socket (task->cfg->deliver_host, un, FALSE);
        }
        else {
-               sock = make_socket (&task->cfg->deliver_addr, task->cfg->deliver_port);
+               sock = make_tcp_socket (&task->cfg->deliver_addr, task->cfg->deliver_port, FALSE);
        }
        if (sock == -1) {
                msg_warn ("lmtp_deliver_mta: cannot create socket for %s, %s", task->cfg->deliver_host, strerror (errno));
        }
-
-       /* Socket options */
-       setsockopt (sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on));
-       setsockopt (sock, SOL_SOCKET, SO_REUSEADDR, (void *)&on, sizeof(on));
-       linger.l_onoff = 1;
-       linger.l_linger = 2;
-       setsockopt (sock, SOL_SOCKET, SO_LINGER, (void *)&linger, sizeof(linger));
        
        cd = memory_pool_alloc (task->task_pool, sizeof (struct mta_callback_data));
        cd->task = task;

diff --git a/src/main.c b/src/main.c
index da142d6a77a052c48e1ecc320d4b58e0f29d5831..1e9ba67889318acfb9bf7a820fa87e319c862155 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -354,14 +354,14 @@ main (int argc, char **argv, char **env)
        sigprocmask(SIG_BLOCK, &signals.sa_mask, NULL);
 
        if (rspamd->cfg->bind_family == AF_INET) {
-               if ((listen_sock = make_socket (&rspamd->cfg->bind_addr, rspamd->cfg->bind_port)) == -1) {
+               if ((listen_sock = make_tcp_socket (&rspamd->cfg->bind_addr, rspamd->cfg->bind_port, TRUE)) == -1) {
                        msg_err ("main: cannot create tcp listen socket. %s", strerror (errno));
                        exit(-errno);
                }
        }
        else {
                un_addr = (struct sockaddr_un *) g_malloc (sizeof (struct sockaddr_un));
-               if (!un_addr || (listen_sock = make_unix_socket (rspamd->cfg->bind_host, un_addr)) == -1) {
+               if (!un_addr || (listen_sock = make_unix_socket (rspamd->cfg->bind_host, un_addr, TRUE)) == -1) {
                        msg_err ("main: cannot create unix listen socket. %s", strerror (errno));
                        exit(-errno);
                }

diff --git a/src/plugins/surbl.c b/src/plugins/surbl.c
index 1e466719c17db05bf93ddcc102f3473c4203cc6a..6798d17e4d4946195eabf70a92548fa456c53fc4 100644 (file)
--- a/src/plugins/surbl.c
+++ b/src/plugins/surbl.c
@@ -596,33 +596,19 @@ redirector_callback (int fd, short what, void *arg)
 static void
 register_redirector_call (struct uri *url, struct worker_task *task) 
 {
-       struct sockaddr_in sc;
-       int ofl, r, s;
+       int s;
        struct redirector_param *param;
        struct timeval timeout;
 
-       bzero (&sc, sizeof (struct sockaddr_in *));
-       sc.sin_family = AF_INET;
-       sc.sin_port = htons (surbl_module_ctx->redirector_port);
-       memcpy (&sc.sin_addr, &surbl_module_ctx->redirector_addr, sizeof (struct in_addr));
-
-       s = socket (PF_INET, SOCK_STREAM, 0);
+       s = make_tcp_socket (&surbl_module_ctx->redirector_addr, htons (surbl_module_ctx->redirector_port), FALSE);
 
        if (s == -1) {
-               msg_info ("register_redirector_call: socket() failed: %s", strerror (errno));
+               msg_info ("register_redirector_call: cannot create tcp socket failed: %s", strerror (errno));
+               task->save.saved --;
+               make_surbl_requests (url, task);
                return; 
        }
 
-       /* set nonblocking */
-    ofl = fcntl (s, F_GETFL, 0);
-    fcntl (s, F_SETFL, ofl | O_NONBLOCK);
-       
-       if ((r = connect (s, (struct sockaddr*)&sc, sizeof (struct sockaddr_in))) == -1) {
-               if (errno != EINPROGRESS) {
-                       close (s);
-                       msg_info ("register_redirector_call: connect() failed: %s", strerror (errno));
-               }
-       }
        param = memory_pool_alloc (task->task_pool, sizeof (struct redirector_param));
        param->url = url;
        param->task = task;

diff --git a/src/util.c b/src/util.c
index fac109c78cb06a2196775e84eee552eabd293670..6a107436ad509e093957adde26fa2c627f799884 100644 (file)
--- a/src/util.c
+++ b/src/util.c
@@ -39,10 +39,10 @@ event_make_socket_nonblocking (int fd)
 }
 
 int
-make_socket (struct in_addr *addr, u_short port)
+make_tcp_socket (struct in_addr *addr, u_short port, gboolean is_server)
 {
        struct linger linger;
-       int fd, on = 1, r;
+       int fd, on = 1, r, optlen, s_error;
        int serrno;
        struct sockaddr_in sin;
        
@@ -55,45 +55,97 @@ make_socket (struct in_addr *addr, u_short port)
        if (event_make_socket_nonblocking(fd) < 0) {
                goto out;
        }
-
-       if (fcntl(fd, F_SETFD, 1) == -1) {
+       
+       /* Set close on exec */
+       if (fcntl (fd, F_SETFD, FD_CLOEXEC) == -1) {
                goto out;
        }
        
        /* Socket options */
-       setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on));
-       setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void *)&on, sizeof(on));
+       setsockopt (fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on));
+       setsockopt (fd, SOL_SOCKET, SO_REUSEADDR, (void *)&on, sizeof(on));
        linger.l_onoff = 1;
        linger.l_linger = 5;
-       setsockopt(fd, SOL_SOCKET, SO_LINGER, (void *)&linger, sizeof(linger));
+       setsockopt (fd, SOL_SOCKET, SO_LINGER, (void *)&linger, sizeof(linger));
 
        /* Bind options */
        sin.sin_family = AF_INET;
        sin.sin_port = htons (port);
        sin.sin_addr.s_addr = addr->s_addr;
        
-       r = bind(fd, (struct sockaddr *)&sin, sizeof (struct sockaddr_in));
+       if (is_server) {
+               r = bind (fd, (struct sockaddr *)&sin, sizeof (struct sockaddr_in));
+       }
+       else {
+               r = connect (fd, (struct sockaddr *)&sin, sizeof (struct sockaddr_in));
+       }
 
        if (r == -1) {
                if (errno != EINPROGRESS) {
                        goto out;
                }
        }
+       else {
+               /* Still need to check SO_ERROR on socket */
+               optlen = sizeof(s_error);
+               getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&s_error, &optlen);
+               if (s_error) {
+                       errno = s_error;
+                       goto out;
+               }
+       }
+
 
        return (fd);
 
  out:
        serrno = errno;
-       close(fd);
+       close (fd);
        errno = serrno;
        return (-1);
 }
 
 int
-make_unix_socket (const char *path, struct sockaddr_un *addr)
+accept_from_socket (int listen_sock, struct sockaddr *addr, socklen_t *len)
+{
+       struct linger linger;
+       int nfd, on = 1;
+       int serrno;
+
+       if ((nfd = accept (listen_sock, addr, len)) == -1) {
+               return -1;
+       }
+       if (event_make_socket_nonblocking(nfd) < 0) {
+               goto out;
+       }
+       
+       /* Set close on exec */
+       if (fcntl (nfd, F_SETFD, FD_CLOEXEC) == -1) {
+               goto out;
+       }
+
+       /* Socket options */
+       setsockopt (nfd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on));
+       setsockopt (nfd, SOL_SOCKET, SO_REUSEADDR, (void *)&on, sizeof(on));
+       linger.l_onoff = 1;
+       linger.l_linger = 2;
+       setsockopt (nfd, SOL_SOCKET, SO_LINGER, (void *)&linger, sizeof(linger));
+
+       return (nfd);
+
+ out:
+       serrno = errno;
+       close (nfd);
+       errno = serrno;
+       return (-1);
+
+}
+
+int
+make_unix_socket (const char *path, struct sockaddr_un *addr, gboolean is_server)
 {
        size_t len = strlen (path);
-       int sock;
+       int fd, s_error, r, optlen, serrno;
 
        if (len > sizeof (addr->sun_path) - 1) return -1;
        
@@ -105,13 +157,50 @@ make_unix_socket (const char *path, struct sockaddr_un *addr)
        
        strncpy (addr->sun_path, path, len);
        
-       sock = socket (PF_LOCAL, SOCK_STREAM, 0);
+       fd = socket (PF_LOCAL, SOCK_STREAM, 0);
+       
+       if (fd == -1) {
+               return -1;
+       }
+
+       if (event_make_socket_nonblocking(fd) < 0) {
+               goto out;
+       }
+       
+       /* Set close on exec */
+       if (fcntl (fd, F_SETFD, FD_CLOEXEC) == -1) {
+               goto out;
+       }
+       if (is_server) {
+               r = bind (fd, (struct sockaddr *)&sin, sizeof (struct sockaddr_in));
+       }
+       else {
+               r = connect (fd, (struct sockaddr *)&sin, sizeof (struct sockaddr_in));
+       }
 
-       if (sock != -1) {
-               if (bind (sock, (struct sockaddr *) addr, sizeof (struct sockaddr_un)) == -1) return -1;
+       if (r == -1) {
+               if (errno != EINPROGRESS) {
+                       goto out;
+               }
+       }
+       else {
+               /* Still need to check SO_ERROR on socket */
+               optlen = sizeof(s_error);
+               getsockopt (fd, SOL_SOCKET, SO_ERROR, (void *)&s_error, &optlen);
+               if (s_error) {
+                       errno = s_error;
+                       goto out;
+               }
        }
 
-       return sock;
+
+       return (fd);
+
+ out:
+       serrno = errno;
+       close (fd);
+       errno = serrno;
+       return (-1);
 }
 
 void 

diff --git a/src/util.h b/src/util.h
index 69ae0fe92c69d11682bf3a16fcd829c9b53215ec..f0764b960ff7f5f62997fc4b84a37ab9b3db7c5e 100644 (file)
--- a/src/util.h
+++ b/src/util.h
@@ -6,10 +6,12 @@
 
 struct config_file;
 
-/* Create socket and bind it to specified address and port */
-int make_socket(struct in_addr *, u_short );
-/* Create and bind unix socket */
-int make_unix_socket (const char *, struct sockaddr_un *);
+/* Create socket and bind or connect it to specified address and port */
+int make_tcp_socket (struct in_addr *, u_short, gboolean is_server);
+/* Accept from socket */
+int accept_from_socket (int listen_sock, struct sockaddr *addr, socklen_t *len);
+/* Create and bind or connect unix socket */
+int make_unix_socket (const char *, struct sockaddr_un *, gboolean is_server);
 /* Parse command line arguments using getopt (3) */
 void read_cmd_line (int , char **, struct config_file *);
 /* Write pid to file */

diff --git a/src/worker.c b/src/worker.c
index dee49ef4df2e3e168ba0edc82f8c30316cf599f0..8e1225024dd6666e04f07f19a3f2c79c59b4e877 100644 (file)
--- a/src/worker.c
+++ b/src/worker.c
@@ -215,22 +215,12 @@ accept_socket (int fd, short what, void *arg)
        struct sockaddr_storage ss;
        struct worker_task *new_task;
        socklen_t addrlen = sizeof(ss);
-       int nfd, on = 1;
-       struct linger linger;
+       int nfd;
 
-       if ((nfd = accept (fd, (struct sockaddr *)&ss, &addrlen)) == -1) {
+       if ((nfd = accept_from_socket (fd, (struct sockaddr *)&ss, &addrlen)) == -1) {
+               msg_warn ("accept_socket: accept failed: %s", strerror (errno));
                return;
        }
-       if (event_make_socket_nonblocking(fd) < 0) {
-               return;
-       }
-
-       /* Socket options */
-       setsockopt (nfd, SOL_SOCKET, SO_KEEPALIVE, (void *)&on, sizeof(on));
-       setsockopt (nfd, SOL_SOCKET, SO_REUSEADDR, (void *)&on, sizeof(on));
-       linger.l_onoff = 1;
-       linger.l_linger = 2;
-       setsockopt (nfd, SOL_SOCKET, SO_LINGER, (void *)&linger, sizeof(linger));
        
        new_task = g_malloc (sizeof (struct worker_task));