[PATCH 07/11] coio: fix file descriptor leak on accept
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Nov 30 18:39:39 MSK 2018
coio_accept() calls evio_setsockopt_client, which
throws an exception and just accepted socket leaks.
Yes, server socket is protected, but not new client
socket.
---
src/coio.cc | 7 +++++--
src/evio.cc | 49 ++++++++++++++++++++++++++++---------------------
src/evio.h | 2 +-
3 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/src/coio.cc b/src/coio.cc
index 49ac10b70..575bae712 100644
--- a/src/coio.cc
+++ b/src/coio.cc
@@ -254,8 +254,11 @@ coio_accept(struct ev_io *coio, struct sockaddr *addr,
int fd = sio_accept(coio->fd, addr, &addrlen,
&is_error_fatal);
if (fd >= 0) {
- evio_setsockopt_client(fd, addr->sa_family,
- SOCK_STREAM);
+ if (evio_setsockopt_client(fd, addr->sa_family,
+ SOCK_STREAM) != 0) {
+ close(fd);
+ diag_raise();
+ }
return fd;
}
if (is_error_fatal)
diff --git a/src/evio.cc b/src/evio.cc
index 4b7d37281..25f699bab 100644
--- a/src/evio.cc
+++ b/src/evio.cc
@@ -64,14 +64,17 @@ void
evio_socket(struct ev_io *coio, int domain, int type, int protocol)
{
assert(coio->fd == -1);
- /* Don't leak fd if setsockopt fails. */
coio->fd = sio_socket(domain, type, protocol);
if (coio->fd < 0)
diag_raise();
- evio_setsockopt_client(coio->fd, domain, type);
+ if (evio_setsockopt_client(coio->fd, domain, type) != 0) {
+ close(coio->fd);
+ coio->fd = -1;
+ diag_raise();
+ }
}
-static void
+static int
evio_setsockopt_keepalive(int fd)
{
int on = 1;
@@ -79,9 +82,8 @@ evio_setsockopt_keepalive(int fd)
* SO_KEEPALIVE to ensure connections don't hang
* around for too long when a link goes away.
*/
- if (sio_setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE,
- &on, sizeof(on)))
- diag_raise();
+ if (sio_setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &on, sizeof(on)) != 0)
+ return -1;
#ifdef __linux__
/*
* On Linux, we are able to fine-tune keepalive
@@ -90,44 +92,46 @@ evio_setsockopt_keepalive(int fd)
*/
int keepcnt = 5;
if (sio_setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, &keepcnt,
- sizeof(int)))
- diag_raise();
+ sizeof(int)) != 0)
+ return -1;
int keepidle = 30;
if (sio_setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &keepidle,
- sizeof(int)))
- diag_raise();
+ sizeof(int)) != 0)
+ return -1;
int keepintvl = 60;
if (sio_setsockopt(fd, IPPROTO_TCP, TCP_KEEPINTVL, &keepintvl,
- sizeof(int)))
- diag_raise();
+ sizeof(int)) != 0)
+ return -1;
#endif
+ return 0;
}
/** Set common client socket options. */
-void
+int
evio_setsockopt_client(int fd, int family, int type)
{
int on = 1;
- /* In case this throws, the socket is not leaked. */
if (sio_setfl(fd, O_NONBLOCK) != 0)
- diag_raise();
+ return -1;
if (type == SOCK_STREAM && family != AF_UNIX) {
/*
* SO_KEEPALIVE to ensure connections don't hang
* around for too long when a link goes away.
*/
- evio_setsockopt_keepalive(fd);
+ if (evio_setsockopt_keepalive(fd) != 0)
+ return -1;
/*
* Lower latency is more important than higher
* bandwidth, and we usually write entire
* request/response in a single syscall.
*/
if (sio_setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
- &on, sizeof(on)))
- diag_raise();
+ &on, sizeof(on)) != 0)
+ return -1;
}
+ return 0;
}
/** Set options for server sockets. */
@@ -150,8 +154,9 @@ evio_setsockopt_server(int fd, int family, int type)
if (sio_setsockopt(fd, SOL_SOCKET, SO_LINGER,
&linger, sizeof(linger)))
diag_raise();
- if (type == SOCK_STREAM && family != AF_UNIX)
- evio_setsockopt_keepalive(fd);
+ if (type == SOCK_STREAM && family != AF_UNIX &&
+ evio_setsockopt_keepalive(fd) != 0)
+ diag_raise();
}
static inline const char *
@@ -191,7 +196,9 @@ evio_service_accept_cb(ev_loop * /* loop */, ev_io *watcher,
return;
}
/* set common client socket options */
- evio_setsockopt_client(fd, service->addr.sa_family, SOCK_STREAM);
+ if (evio_setsockopt_client(fd, service->addr.sa_family,
+ SOCK_STREAM) != 0)
+ diag_raise();
/*
* Invoke the callback and pass it the accepted
* socket.
diff --git a/src/evio.h b/src/evio.h
index f6c3a3a3e..9f80e84a5 100644
--- a/src/evio.h
+++ b/src/evio.h
@@ -150,7 +150,7 @@ evio_timeout_update(ev_loop *loop, ev_tstamp start, ev_tstamp *delay)
*delay = (elapsed >= *delay) ? 0 : *delay - elapsed;
}
-void
+int
evio_setsockopt_client(int fd, int family, int type);
#endif /* TARANTOOL_EVIO_H_INCLUDED */
--
2.17.2 (Apple Git-113)
More information about the Tarantool-patches
mailing list