From 9c0c55198c2406fea6be189e1ec6b3ac3cc565fc Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 24 Aug 2016 17:52:48 +0200 Subject: ipcpd: Revised locking The state lock was reverted to an rwlock to avoid interference of management functions with the fast path. IPCPs now close without calling unsafe functions in the signal handler. --- src/ipcpd/ipcp.c | 122 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 83 insertions(+), 39 deletions(-) (limited to 'src/ipcpd/ipcp.c') diff --git a/src/ipcpd/ipcp.c b/src/ipcpd/ipcp.c index 1c4fab94..87bf9e51 100644 --- a/src/ipcpd/ipcp.c +++ b/src/ipcpd/ipcp.c @@ -22,6 +22,7 @@ #include #include +#include #define OUROBOROS_PREFIX "ipcpd/ipcp" #include @@ -44,7 +45,8 @@ struct ipcp * ipcp_instance_create() i->irmd_fd = -1; i->state = IPCP_INIT; - pthread_mutex_init(&i->state_lock, NULL); + pthread_rwlock_init(&i->state_lock, NULL); + pthread_mutex_init(&i->state_mtx, NULL); pthread_condattr_init(&cattr); pthread_condattr_setclock(&cattr, PTHREAD_COND_CLOCK); pthread_cond_init(&i->state_cond, &cattr); @@ -52,14 +54,65 @@ struct ipcp * ipcp_instance_create() return i; } -void ipcp_state_change(struct ipcp * ipcp, - enum ipcp_state state) +void ipcp_set_state(struct ipcp * ipcp, + enum ipcp_state state) { if (ipcp == NULL) return; + pthread_mutex_lock(&ipcp->state_mtx); + ipcp->state = state; + pthread_cond_broadcast(&ipcp->state_cond); + pthread_mutex_unlock(&ipcp->state_mtx); +} + +enum ipcp_state ipcp_get_state(struct ipcp * ipcp) +{ + enum ipcp_state state; + + if (ipcp == NULL) + return IPCP_NULL; + + pthread_mutex_lock(&ipcp->state_mtx); + + state = ipcp->state; + + pthread_mutex_unlock(&ipcp->state_mtx); + + return state; +} + +int ipcp_wait_state(struct ipcp * ipcp, + enum ipcp_state state, + const struct timespec * timeout) +{ + struct timespec abstime; + + clock_gettime(PTHREAD_COND_CLOCK, &abstime); + ts_add(&abstime, timeout, &abstime); + + pthread_mutex_lock(&ipcp->state_mtx); + + while (ipcp->state != state && ipcp->state != IPCP_SHUTDOWN) { + int ret; + if (timeout == NULL) + ret = pthread_cond_wait(&ipcp->state_cond, + &ipcp->state_mtx); + else + ret = pthread_cond_timedwait(&ipcp->state_cond, + &ipcp->state_mtx, + &abstime); + if (ret) { + pthread_mutex_unlock(&ipcp->state_mtx); + return -ret; + } + } + + pthread_mutex_unlock(&ipcp->state_mtx); + + return 0; } int ipcp_parse_arg(int argc, char * argv[]) @@ -101,22 +154,6 @@ int ipcp_parse_arg(int argc, char * argv[]) return 0; } -static void close_ptr(void * o) -{ - char * name = ipcp_sock_path(getpid()); - close(*((int *) o)); - - if (unlink(name)) - LOG_DBG("Could not unlink %s.", name); - - free(name); -} - -static void clean_msg(void * msg) -{ - ipcp_msg__free_unpacked(msg, NULL); -} - void * ipcp_main_loop(void * o) { int lsockfd; @@ -135,9 +172,13 @@ void * ipcp_main_loop(void * o) char * sock_path; char * msg_name_dup; - struct timeval tv = {(SOCKET_TIMEOUT / 1000), + struct timeval tv = {(IPCP_ACCEPT_TIMEOUT / 1000), + (IPCP_ACCEPT_TIMEOUT % 1000) * 1000}; + + struct timeval ltv = {(SOCKET_TIMEOUT / 1000), (SOCKET_TIMEOUT % 1000) * 1000}; + if (_ipcp == NULL) { LOG_ERR("Invalid ipcp struct."); return (void *) 1; @@ -150,28 +191,33 @@ void * ipcp_main_loop(void * o) sockfd = server_socket_open(sock_path); if (sockfd < 0) { LOG_ERR("Could not open server socket."); + free(sock_path); return (void *) 1; } - free(sock_path); - - pthread_cleanup_push(close_ptr, (void *) &sockfd); + if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, + (void *) &tv, sizeof(tv))) + LOG_WARN("Failed to set timeout on socket."); while (true) { + pthread_rwlock_rdlock(&_ipcp->state_lock); + if (ipcp_get_state(_ipcp) == IPCP_SHUTDOWN) { + pthread_rwlock_unlock(&_ipcp->state_lock); + break; + } + + pthread_rwlock_unlock(&_ipcp->state_lock); + ret_msg.code = IPCP_MSG_CODE__IPCP_REPLY; lsockfd = accept(sockfd, 0, 0); - if (lsockfd < 0) { - LOG_ERR("Cannot accept new connection"); - break; - } + if (lsockfd < 0) + continue; if (setsockopt(lsockfd, SOL_SOCKET, SO_RCVTIMEO, - (void *) &tv, sizeof(tv))) + (void *) <v, sizeof(ltv))) LOG_WARN("Failed to set timeout on socket."); - pthread_cleanup_push(close_ptr, (void *) &lsockfd); - count = read(lsockfd, buf, IPCP_MSG_BUF_SIZE); if (count <= 0) { LOG_ERR("Failed to read from socket"); @@ -185,8 +231,6 @@ void * ipcp_main_loop(void * o) continue; } - pthread_cleanup_push(clean_msg, (void *) msg); - switch (msg->code) { case IPCP_MSG_CODE__IPCP_BOOTSTRAP: if (_ipcp->ops->ipcp_bootstrap == NULL) { @@ -201,7 +245,6 @@ void * ipcp_main_loop(void * o) ret_msg.result = -1; break; } - if (conf_msg->ipcp_type == IPCP_NORMAL) { conf.addr_size = conf_msg->addr_size; conf.cep_id_size = conf_msg->cep_id_size; @@ -218,7 +261,6 @@ void * ipcp_main_loop(void * o) conf.ip_addr = conf_msg->ip_addr; conf.dns_addr = conf_msg->dns_addr; } - if (conf_msg->ipcp_type == IPCP_SHIM_ETH_LLC) conf.if_name = conf_msg->if_name; @@ -295,8 +337,7 @@ void * ipcp_main_loop(void * o) break; } - pthread_cleanup_pop(true); - pthread_cleanup_pop(false); + ipcp_msg__free_unpacked(msg, NULL); buffer.len = ipcp_msg__get_packed_size(&ret_msg); if (buffer.len == 0) { @@ -321,10 +362,13 @@ void * ipcp_main_loop(void * o) free(buffer.data); close(lsockfd); - } - pthread_cleanup_pop(true); + close(sockfd); + if (unlink(sock_path)) + LOG_DBG("Could not unlink %s.", sock_path); + + free(sock_path); - return NULL; + return (void *) 0; } -- cgit v1.2.3