From: pcarana Date: Tue, 28 Jul 2020 23:16:35 +0000 (-0500) Subject: Change the way that the server handles client connections, fix memleak X-Git-Tag: v1.4.0~10 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=128c56026101847e4e7311a516a86f92afa40e8a;p=thirdparty%2FFORT-validator.git Change the way that the server handles client connections, fix memleak +Using a thread for each server socket worked ok, but at a great cost since each thread can increase memory consumption; instead, use non-blocking sockets and select() to poll for client connections on each configured server socket. +Fix memory leak when the incidence 'stale CRL' returns error. --- diff --git a/src/object/certificate.c b/src/object/certificate.c index 5fee91f0..fc64054a 100644 --- a/src/object/certificate.c +++ b/src/object/certificate.c @@ -990,7 +990,7 @@ certificate_validate_chain(X509 *cert, STACK_OF(X509_CRL) *crls) if (error) { if (error == X509_V_ERR_CRL_HAS_EXPIRED) { if (incidence(INID_CRL_STALE, "CRL is stale/expired")) - return -EINVAL; + goto abort; X509_STORE_CTX_free(ctx); return verify_cert_crl_stale(state, cert, crls); diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c index f595552b..469a33df 100644 --- a/src/rtr/rtr.c +++ b/src/rtr/rtr.c @@ -1,6 +1,7 @@ #include "rtr.h" #include +#include #include #include #include @@ -95,6 +96,7 @@ create_server_socket(char const *hostname, char const *service, int *result) struct addrinfo *addrs; struct addrinfo *addr; unsigned long port; + int flags; int reuse; int fd; /* "file descriptor" */ int error; @@ -118,6 +120,20 @@ create_server_socket(char const *hostname, char const *service, int *result) continue; } + flags = fcntl(fd, F_GETFL); + if (flags == -1) { + pr_op_errno(errno, "fcntl() to get flags failed"); + continue; + } + + /* Non-block to allow listening on all server sockets */ + flags |= O_NONBLOCK; + + if (fcntl(fd, F_SETFL, flags) == -1) { + pr_op_errno(errno, "fcntl() to set flags failed"); + continue; + } + /* enable SO_REUSEADDR */ if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(int)) < 0) { @@ -429,17 +445,33 @@ client_thread_cb(void *arg) return NULL; } +static void +init_fdset(struct server_fds *fds, fd_set *fdset) +{ + struct fd_node *node; + + FD_ZERO (fdset); + SLIST_FOREACH(node, fds, next) + FD_SET (node->id, fdset); + +} + /* * Waits for client connections and spawns threads to handle them. */ static int -handle_client_connections(int server_fd) +handle_client_connections(struct server_fds *fds) { + struct fd_node *head_node; struct sigaction ign; struct sockaddr_storage client_addr; struct thread_param *param; + struct timeval select_time; socklen_t sizeof_client_addr; + fd_set readfds; + int last_server_fd; int client_fd; + int fd; int error; /* Ignore SIGPIPES, they're handled apart */ @@ -448,105 +480,84 @@ handle_client_connections(int server_fd) sigemptyset(&ign.sa_mask); sigaction(SIGPIPE, &ign, NULL); - listen(server_fd, config_get_server_queue()); + head_node = SLIST_FIRST(fds); + last_server_fd = head_node->id; + + SLIST_FOREACH(head_node, fds, next) { + error = listen(head_node->id, config_get_server_queue()); + if (error) + return pr_op_errno(errno, + "Couldn't listen on server socket."); + } sizeof_client_addr = sizeof(client_addr); - pr_op_debug("Waiting for client connections at server FD %d...", - server_fd); + pr_op_debug("Waiting for client connections at server..."); do { - client_fd = accept(server_fd, (struct sockaddr *) &client_addr, - &sizeof_client_addr); - switch (handle_accept_result(client_fd, errno)) { - case VERDICT_SUCCESS: - break; - case VERDICT_RETRY: - continue; - case VERDICT_EXIT: - return -EINVAL; - } + /* Look for connections every .2 seconds*/ + select_time.tv_sec = 0; + select_time.tv_usec = 200000; - print_client_addr(&client_addr, "accepted", client_fd); + init_fdset(fds, &readfds); - /* - * Note: My gut says that errors from now on (even the unknown - * ones) should be treated as temporary; maybe the next - * accept() will work. - * So don't interrupt the thread when this happens. - */ - - param = malloc(sizeof(struct thread_param)); - if (param == NULL) { - /* No error response PDU on memory allocation. */ - pr_op_err("Couldn't create thread parameters struct"); - close(client_fd); + if (select(last_server_fd + 1, &readfds, NULL, NULL, + &select_time) == -1) { + pr_op_errno(errno, "Monitoring server sockets"); continue; } - param->fd = client_fd; - param->addr = client_addr; - - error = pthread_create(¶m->tid, NULL, client_thread_cb, - param); - if (error && error != EAGAIN) - /* Error with min RTR version */ - err_pdu_send_internal_error(client_fd, RTR_V0); - if (error) { - pr_op_errno(error, "Could not spawn the client's thread"); - close(client_fd); - free(param); - } + for (fd = 0; fd < (last_server_fd + 1); fd++) { + if (!FD_ISSET (fd, &readfds)) + continue; + + /* Accept the connection */ + client_fd = accept(fd, (struct sockaddr *) &client_addr, + &sizeof_client_addr); + switch (handle_accept_result(client_fd, errno)) { + case VERDICT_SUCCESS: + break; + case VERDICT_RETRY: + continue; + case VERDICT_EXIT: + return -EINVAL; + } + + print_client_addr(&client_addr, "accepted", client_fd); + + /* + * Note: My gut says that errors from now on (even the + * unknown ones) should be treated as temporary; maybe + * the next accept() will work. + * So don't interrupt the thread when this happens. + */ + + param = malloc(sizeof(struct thread_param)); + if (param == NULL) { + /* No error PDU on memory allocation. */ + pr_enomem(); + close(client_fd); + continue; + } + param->fd = client_fd; + param->addr = client_addr; + + error = pthread_create(¶m->tid, NULL, + client_thread_cb, param); + if (error && error != EAGAIN) + /* Error with min RTR version */ + err_pdu_send_internal_error(client_fd, RTR_V0); + if (error) { + pr_op_errno(error, + "Could not spawn the client's thread"); + close(client_fd); + free(param); + } + } } while (true); return 0; /* Unreachable. */ } -static void * -server_thread_cb(void *arg) -{ - int *server_fd = (int *)(arg); - handle_client_connections(*server_fd); - return NULL; -} - -static void -server_fd_stop_threads(struct server_fds *fds) -{ - struct fd_node *node; - - SLIST_FOREACH(node, fds, next) { - if (node ->tid < 0) - continue; - close_thread(node->tid, "RTR server"); - node->tid = -1; - } -} - -static int -__handle_client_connections(struct server_fds *fds) -{ - struct fd_node *node; - int error; - - SLIST_FOREACH(node, fds, next) { - error = pthread_create(&node->tid, NULL, server_thread_cb, - &node->id); - if (error) { - server_fd_stop_threads(fds); - return error; - } - } - - SLIST_FOREACH(node, fds, next) { - error = pthread_join(node->tid, NULL); - if (error) - pr_crit("pthread_join() threw %d on an RTR server thread.", - error); - } - - return 0; -} - /* * Receive @arg to be called as a clients_foreach_cb */ @@ -609,7 +620,7 @@ rtr_listen(void) if (error) goto revert_server_sockets; - error = __handle_client_connections(&fds); + error = handle_client_connections(&fds); end_clients(); updates_daemon_destroy();