From a3df50f31fcfd9d76bd5cb8c97440a9092dafdf2 Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Sat, 9 Sep 2017 00:35:42 +0300 Subject: [PATCH 1/5] sslh-select: fix connections with deferred data after connect_queue Previously, if some data was still deferred after the connect_queue call, the server side of the connection would never start being monitored for reads, while the client side kept being monitored and new data from the client could be sent to the server before the previously deferred data. --- sslh-select.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sslh-select.c b/sslh-select.c index db05645..765284e 100644 --- a/sslh-select.c +++ b/sslh-select.c @@ -140,9 +140,9 @@ int connect_queue(struct connection *cnx, fd_set *fds_r, fd_set *fds_w) flush_deferred(q); if (q->deferred_data) { FD_SET(q->fd, fds_w); - } else { - FD_SET(q->fd, fds_r); + FD_CLR(cnx->q[0].fd, fds_r); } + FD_SET(q->fd, fds_r); return q->fd; } else { tidy_connection(cnx, fds_r, fds_w); From 684c9afcc6f3c4e6976c25bafd70b3507840f861 Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Sat, 9 Sep 2017 00:44:24 +0300 Subject: [PATCH 2/5] sslh-select: actually close socket on error in accept_new_connection Previously, it was leaked (and the client was left waiting for a timeout). --- sslh-select.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sslh-select.c b/sslh-select.c index 765284e..146e1ca 100644 --- a/sslh-select.c +++ b/sslh-select.c @@ -93,11 +93,16 @@ int accept_new_connection(int listen_socket, struct connection *cnx[], int* cnx_ in_socket = accept(listen_socket, 0, 0); CHECK_RES_RETURN(in_socket, "accept"); - if (!fd_is_in_range(in_socket)) + if (!fd_is_in_range(in_socket)) { + close(in_socket); return -1; + } res = set_nonblock(in_socket); - if (res == -1) return -1; + if (res == -1) { + close(in_socket); + return -1; + } /* Find an empty slot */ for (free = 0; (free < *cnx_size) && ((*cnx)[free].q[0].fd != -1); free++) { @@ -109,6 +114,7 @@ int accept_new_connection(int listen_socket, struct connection *cnx[], int* cnx_ new = realloc(*cnx, (*cnx_size + cnx_num_alloc) * sizeof((*cnx)[0])); if (!new) { log_message(LOG_ERR, "unable to realloc -- dropping connection\n"); + close(in_socket); return -1; } *cnx = new; From b56f302b85e1de5f3b44ddc545cf7696c8f4c8e3 Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Sat, 9 Sep 2017 00:58:21 +0300 Subject: [PATCH 3/5] sslh-select: simplify some code --- sslh-select.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sslh-select.c b/sslh-select.c index 146e1ca..8d279bb 100644 --- a/sslh-select.c +++ b/sslh-select.c @@ -254,15 +254,12 @@ void main_loop(int listen_sockets[], int num_addr_listen) for (i = 0; i < num_addr_listen; i++) { if (FD_ISSET(listen_sockets[i], &readfds)) { in_socket = accept_new_connection(listen_sockets[i], &cnx, &num_cnx); - if (in_socket != -1) - num_probing++; - if (in_socket > 0) { + num_probing++; FD_SET(in_socket, &fds_r); if (in_socket >= max_fd) max_fd = in_socket + 1;; } - FD_CLR(listen_sockets[i], &readfds); } } From b7fafb5039036473a210234040ad8a9119b86d7d Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Sat, 9 Sep 2017 00:59:55 +0300 Subject: [PATCH 4/5] sslh-select: invoke FD_CLR on fd before closing fd POSIX requires the fd argument to any FD_ macro to be valid. --- sslh-select.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sslh-select.c b/sslh-select.c index 8d279bb..2bee1e2 100644 --- a/sslh-select.c +++ b/sslh-select.c @@ -61,9 +61,9 @@ int tidy_connection(struct connection *cnx, fd_set *fds, fd_set *fds2) if (verbose) fprintf(stderr, "closing fd %d\n", cnx->q[i].fd); - close(cnx->q[i].fd); FD_CLR(cnx->q[i].fd, fds); FD_CLR(cnx->q[i].fd, fds2); + close(cnx->q[i].fd); if (cnx->q[i].deferred_data) free(cnx->q[i].deferred_data); } From 60b11e4964c23d2946f829fdb6da8d9b5284433d Mon Sep 17 00:00:00 2001 From: Oleg Oshmyan Date: Sat, 9 Sep 2017 15:43:11 +0300 Subject: [PATCH 5/5] Fix defer_write when deferred_data != begin_deferred_data I think this currently never happens, but let's not wait until it starts happening and blows up. --- common.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/common.c b/common.c index be68338..b2daef6 100644 --- a/common.c +++ b/common.c @@ -6,6 +6,7 @@ #define SYSLOG_NAMES #define _GNU_SOURCE +#include #include #include @@ -285,17 +286,19 @@ int connect_addr(struct connection *cnx, int fd_from) int defer_write(struct queue *q, void* data, int data_size) { char *p; + ptrdiff_t data_offset = q->deferred_data - q->begin_deferred_data; if (verbose) fprintf(stderr, "**** writing deferred on fd %d\n", q->fd); - p = realloc(q->begin_deferred_data, q->deferred_data_size + data_size); + p = realloc(q->begin_deferred_data, data_offset + q->deferred_data_size + data_size); if (!p) { perror("realloc"); exit(1); } - q->deferred_data = q->begin_deferred_data = p; - p += q->deferred_data_size; + q->begin_deferred_data = p; + q->deferred_data = p + data_offset; + p += data_offset + q->deferred_data_size; q->deferred_data_size += data_size; memcpy(p, data, data_size);