From: Alan T. DeKok Date: Thu, 14 Mar 2013 01:41:32 +0000 (-0400) Subject: Free all listeners when we exit. X-Git-Tag: release_3_0_0_beta1~755 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8673bb889afeeac973fcb3e7442096225ccef147;p=thirdparty%2Ffreeradius-server.git Free all listeners when we exit. Both talloc and valgrind now agree that we leak zero bytes on clean exit --- diff --git a/src/include/radiusd.h b/src/include/radiusd.h index 430e33b5931..7644c6873f3 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -779,7 +779,7 @@ void fr_suid_down_permanent(void); /* listen.c */ void listen_free(rad_listen_t **head); int listen_init(CONF_SECTION *cs, rad_listen_t **head, int spawn_flag); -int proxy_new_listener(home_server *home, int src_port); +rad_listen_t *proxy_new_listener(home_server *home, int src_port); RADCLIENT *client_listener_find(rad_listen_t *listener, const fr_ipaddr_t *ipaddr, int src_port); diff --git a/src/main/command.c b/src/main/command.c index ad76d797fe9..a9079947fcf 100644 --- a/src/main/command.c +++ b/src/main/command.c @@ -2069,17 +2069,15 @@ static void command_socket_free(rad_listen_t *this) { fr_command_socket_t *cmd = this->data; + /* + * If it's a TCP socket, don't do anything. + */ if (cmd->magic != COMMAND_SOCKET_MAGIC) { - listen_socket_t *sock = this->data; - - free(sock->packet); return; } if (!cmd->copy) return; unlink(cmd->copy); - free(cmd->copy); - cmd->copy = NULL; } @@ -2102,7 +2100,7 @@ static int command_socket_parse_unix(CONF_SECTION *cs, rad_listen_t *this) sock->magic = COMMAND_SOCKET_MAGIC; sock->copy = NULL; - if (sock->path) sock->copy = strdup(sock->path); + if (sock->path) sock->copy = talloc_strdup(sock, sock->path); #if defined(HAVE_GETPEEREID) || defined (SO_PEERCRED) if (sock->uid_name) { @@ -2534,9 +2532,7 @@ static int command_write_magic(int newfd, listen_socket_t *sock) int i; fr_cs_buffer_t *co; - co = rad_malloc(sizeof(*co)); - memset(co, 0, sizeof(*co)); - + co = talloc_zero(sock, fr_cs_buffer_t); sock->packet = (void *) co; for (i = 0; i < 16; i++) { @@ -2703,7 +2699,7 @@ static int command_domain_accept(rad_listen_t *listener) /* * Add the new listener. */ - this = listen_alloc(listener->type); + this = listen_alloc(listener, listener->type); if (!this) return 0; /* diff --git a/src/main/dhcpd.c b/src/main/dhcpd.c index 8deac791da3..4b262ba3fbd 100644 --- a/src/main/dhcpd.c +++ b/src/main/dhcpd.c @@ -568,7 +568,7 @@ static int dhcp_socket_parse(CONF_SECTION *cs, rad_listen_t *this) } if (!sock->src_interface && sock->interface) { - sock->src_interface = strdup(sock->interface); + sock->src_interface = talloc_strdup(sock, sock->interface); } cp = cf_pair_find(cs, "src_ipaddr"); @@ -594,7 +594,7 @@ static int dhcp_socket_parse(CONF_SECTION *cs, rad_listen_t *this) client->prefix = 0; client->longname = client->shortname = "dhcp"; client->secret = client->shortname; - client->nastype = strdup("none"); + client->nastype = talloc_strdup(sock, "none"); return 0; } diff --git a/src/main/listen.c b/src/main/listen.c index cac24e2e15c..0548e4862e4 100644 --- a/src/main/listen.c +++ b/src/main/listen.c @@ -81,7 +81,7 @@ typedef struct rad_listen_master_t { rad_listen_decode_t decode; } rad_listen_master_t; -static rad_listen_t *listen_alloc(RAD_LISTEN_TYPE type); +static rad_listen_t *listen_alloc(TALLOC_CTX *ctx, RAD_LISTEN_TYPE type); #ifdef WITH_COMMAND_SOCKET static int command_tcp_recv(rad_listen_t *listener); @@ -623,7 +623,7 @@ static int dual_tcp_accept(rad_listen_t *listener) /* * Add the new listener. */ - this = listen_alloc(listener->type); + this = listen_alloc(listener, listener->type); if (!this) return -1; /* @@ -2440,15 +2440,61 @@ static int listen_bind(rad_listen_t *this) } +static int listener_free(void *ctx) +{ + rad_listen_t *this; + + this = talloc_get_type_abort(ctx, rad_listen_t); + + /* + * Other code may have eaten the FD. + */ + if (this->fd >= 0) close(this->fd); + + if (master_listen[this->type].free) { + master_listen[this->type].free(this); + } + +#ifdef WITH_TCP + if ((this->type == RAD_LISTEN_AUTH) +#ifdef WITH_ACCT + || (this->type == RAD_LISTEN_ACCT) +#endif +#ifdef WITH_PROXY + || (this->type == RAD_LISTEN_PROXY) +#endif +#ifdef WITH_COMMAND_SOCKET + || ((this->type == RAD_LISTEN_COMMAND) && + (((fr_command_socket_t *) this->data)->magic != COMMAND_SOCKET_MAGIC)) +#endif + ) { + listen_socket_t *sock = this->data; + +#ifdef WITH_TLS + if (sock->request) { + pthread_mutex_destroy(&(sock->mutex)); + request_free(&sock->request); + sock->packet = NULL; + + if (sock->ssn) session_free(sock->ssn); + request_free(&sock->request); + } else +#endif + rad_free(&sock->packet); + } +#endif /* WITH_TCP */ + + return 0; +} + /* * Allocate & initialize a new listener. */ -static rad_listen_t *listen_alloc(RAD_LISTEN_TYPE type) +static rad_listen_t *listen_alloc(TALLOC_CTX *ctx, RAD_LISTEN_TYPE type) { rad_listen_t *this; - this = rad_malloc(sizeof(*this)); - memset(this, 0, sizeof(*this)); + this = talloc_zero(ctx, rad_listen_t); this->type = type; this->recv = master_listen[this->type].recv; @@ -2457,6 +2503,8 @@ static rad_listen_t *listen_alloc(RAD_LISTEN_TYPE type) this->encode = master_listen[this->type].encode; this->decode = master_listen[this->type].decode; + talloc_set_destructor((void *) this, listener_free); + switch (type) { #ifdef WITH_STATS case RAD_LISTEN_NONE: @@ -2474,14 +2522,12 @@ static rad_listen_t *listen_alloc(RAD_LISTEN_TYPE type) #ifdef WITH_COA case RAD_LISTEN_COA: #endif - this->data = rad_malloc(sizeof(listen_socket_t)); - memset(this->data, 0, sizeof(listen_socket_t)); + this->data = talloc_zero(this, listen_socket_t); break; #ifdef WITH_DHCP case RAD_LISTEN_DHCP: - this->data = rad_malloc(sizeof(dhcp_socket_t)); - memset(this->data, 0, sizeof(dhcp_socket_t)); + this->data = talloc_zero(this, dhcp_socket_t); break; #endif @@ -2493,8 +2539,7 @@ static rad_listen_t *listen_alloc(RAD_LISTEN_TYPE type) #ifdef WITH_COMMAND_SOCKET case RAD_LISTEN_COMMAND: - this->data = rad_malloc(sizeof(fr_command_socket_t)); - memset(this->data, 0, sizeof(fr_command_socket_t)); + this->data = talloc_zero(this, fr_command_socket_t); break; #endif @@ -2513,22 +2558,22 @@ static rad_listen_t *listen_alloc(RAD_LISTEN_TYPE type) * Not thread-safe, but all calls to it are protected by the * proxy mutex in event.c */ -int proxy_new_listener(home_server *home, int src_port) +rad_listen_t *proxy_new_listener(home_server *home, int src_port) { rad_listen_t *this; listen_socket_t *sock; char buffer[256]; - if (!home) return 0; + if (!home) return NULL; if ((home->limit.max_connections > 0) && (home->limit.num_connections >= home->limit.max_connections)) { DEBUGW("Home server has too many open connections (%d)", home->limit.max_connections); - return 0; + return NULL; } - this = listen_alloc(RAD_LISTEN_PROXY); + this = listen_alloc(mainconfig.config, RAD_LISTEN_PROXY); sock = this->data; sock->other_ipaddr = home->ipaddr; @@ -2565,7 +2610,7 @@ int proxy_new_listener(home_server *home, int src_port) sock->ssn = tls_new_client_session(home->tls, this->fd); if (!sock->ssn) { listen_free(&this); - return 0; + return NULL; } this->recv = proxy_tls_recv; @@ -2581,7 +2626,7 @@ int proxy_new_listener(home_server *home, int src_port) DEBUG("Failed opening client socket ::%s:: : %s", buffer, fr_strerror()); listen_free(&this); - return 0; + return NULL; } /* @@ -2597,26 +2642,18 @@ int proxy_new_listener(home_server *home, int src_port) radlog(L_ERR, "Failed getting socket name: %s", strerror(errno)); listen_free(&this); - return 0; + return NULL; } if (!fr_sockaddr2ipaddr(&src, sizeof_src, &sock->my_ipaddr, &sock->my_port)) { radlog(L_ERR, "Socket has unsupported address family"); listen_free(&this); - return 0; + return NULL; } } - /* - * Tell the event loop that we have a new FD - */ - if (!event_new_fd(this)) { - listen_free(&this); - return 0; - } - - return 1; + return this; } #endif @@ -2707,7 +2744,7 @@ static rad_listen_t *listen_parse(CONF_SECTION *cs, const char *server) /* * Set up cross-type data. */ - this = listen_alloc(type); + this = listen_alloc(cs, type); this->server = server; this->fd = -1; @@ -2806,11 +2843,11 @@ int listen_init(CONF_SECTION *config, rad_listen_t **head, int spawn_flag) #ifdef WITH_VMPS if (strcmp(progname, "vmpsd") == 0) { - this = listen_alloc(RAD_LISTEN_VQP); + this = listen_alloc(config, RAD_LISTEN_VQP); if (!auth_port) auth_port = 1589; } else #endif - this = listen_alloc(RAD_LISTEN_AUTH); + this = listen_alloc(config, RAD_LISTEN_AUTH); sock = this->data; @@ -2855,7 +2892,7 @@ int listen_init(CONF_SECTION *config, rad_listen_t **head, int spawn_flag) * If we haven't already gotten acct_port from * /etc/services, then make it auth_port + 1. */ - this = listen_alloc(RAD_LISTEN_ACCT); + this = listen_alloc(config, RAD_LISTEN_ACCT); sock = this->data; /* @@ -3087,45 +3124,7 @@ void listen_free(rad_listen_t **head) this = *head; while (this) { rad_listen_t *next = this->next; - - /* - * Other code may have eaten the FD. - */ - if (this->fd >= 0) close(this->fd); - - if (master_listen[this->type].free) { - master_listen[this->type].free(this); - } - -#ifdef WITH_TCP - if ((this->type == RAD_LISTEN_AUTH) -#ifdef WITH_ACCT - || (this->type == RAD_LISTEN_ACCT) -#endif -#ifdef WITH_PROXY - || (this->type == RAD_LISTEN_PROXY) -#endif - ) { - listen_socket_t *sock = this->data; - -#ifdef WITH_TLS - if (sock->request) { - pthread_mutex_destroy(&(sock->mutex)); - request_free(&sock->request); - sock->packet = NULL; - - if (sock->ssn) session_free(sock->ssn); - request_free(&sock->request); - } else -#endif - rad_free(&sock->packet); - - } -#endif /* WITH_TCP */ - - free(this->data); - free(this); - + talloc_free(this); this = next; } diff --git a/src/main/process.c b/src/main/process.c index 1c0b581a187..05b0c268e5d 100644 --- a/src/main/process.c +++ b/src/main/process.c @@ -1689,49 +1689,56 @@ static int insert_into_proxy_hash(REQUEST *request) char buf[128]; int rcode, tries; void *proxy_listener; + rad_listen_t *this; rad_assert(request->proxy != NULL); rad_assert(request->home_server != NULL); rad_assert(proxy_list != NULL); - tries = 1; -retry: + PTHREAD_MUTEX_LOCK(&proxy_mutex); - rcode = fr_packet_list_id_alloc(proxy_list, - request->home_server->proto, - request->proxy, &proxy_listener); + this = proxy_listener = NULL; request->num_proxied_requests = 1; request->num_proxied_responses = 0; - PTHREAD_MUTEX_UNLOCK(&proxy_mutex); - - if (!rcode) { + + for (tries = 0; tries < 2; tries++) { + rcode = fr_packet_list_id_alloc(proxy_list, + request->home_server->proto, + request->proxy, &proxy_listener); + if (rcode > 0) break; + if (tries > 0) break; /* try opening new socket only once */ + #ifdef HAVE_PTHREAD_H - if (proxy_no_new_sockets) return 0; + if (proxy_no_new_sockets) break; #endif - /* - * Also locks the proxy mutex, so we have to call - * it with the mutex unlocked. Some systems - * don't support recursive mutexes. - */ - if (!proxy_new_listener(request->home_server, 0)) { + this = proxy_new_listener(request->home_server, 0); + if (!this) { radlog(L_ERR, "Failed to create a new socket for proxying requests."); - return 0; + break; } request->proxy->src_port = 0; /* Use any new socket */ - - tries++; - if (tries > 2) { - RDEBUG2E("Failed allocating Id for new socket when proxying requests."); - return 0; - } - - goto retry; + proxy_listener = this; } - request->proxy_listener = proxy_listener; + if (!proxy_listener) { + RDEBUG2E("Failed allocating Id for new socket when proxying requests."); + PTHREAD_MUTEX_UNLOCK(&proxy_mutex); + return 0; + } + /* + * Tell the event loop that we have a new FD. It locks + * the socket, so we've got to unlock it here. + */ + PTHREAD_MUTEX_UNLOCK(&proxy_mutex); + if (!event_new_fd(this)) { + listen_free(&this); + return 0; + } PTHREAD_MUTEX_LOCK(&proxy_mutex); + + request->proxy_listener = this; if (!fr_packet_list_insert(proxy_list, &request->proxy)) { fr_packet_list_id_free(proxy_list, request->proxy); PTHREAD_MUTEX_UNLOCK(&proxy_mutex);