From a8c5a9787ba4e8757b16505409c4fbe7d7549e18 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 20:34:56 +0200 Subject: tools: cbr: Fix missing check on ap_init If ap_init() fails, the program should exit. --- src/tools/cbr/cbr.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tools/cbr/cbr.c b/src/tools/cbr/cbr.c index 0cce50db..65783119 100644 --- a/src/tools/cbr/cbr.c +++ b/src/tools/cbr/cbr.c @@ -75,8 +75,10 @@ int main(int argc, char ** argv) bool server = false; - /* FIXME: should be argv[0] */ - ap_init(argv[0]); + if (ap_init(argv[0]) < 0) { + printf("Failed to init.\n"); + exit(EXIT_FAILURE); + } server_settings.interval = 1; /* One second reporting interval */ server_settings.timeout = 1; -- cgit v1.2.3 From c6983c01bcbfff43f1c705c97c041280867d539b Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 20:36:40 +0200 Subject: lib: shm_du_map: Fix memory leak --- src/lib/shm_du_map.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib/shm_du_map.c b/src/lib/shm_du_map.c index a12ef223..d88e5c84 100644 --- a/src/lib/shm_du_map.c +++ b/src/lib/shm_du_map.c @@ -182,7 +182,6 @@ struct shm_du_map * shm_du_map_create() if (shm_base == MAP_FAILED) { LOG_DBGF("Failed to map shared memory."); - if (shm_unlink(SHM_DU_MAP_FILENAME) == -1) LOG_DBGF("Failed to remove invalid shm."); @@ -237,6 +236,7 @@ struct shm_du_map * shm_du_map_open() shm_fd = shm_open(SHM_DU_MAP_FILENAME, O_RDWR, 0666); if (shm_fd < 0) { LOG_DBGF("Failed opening shared memory."); + free(dum); return NULL; } @@ -250,10 +250,9 @@ struct shm_du_map * shm_du_map_open() LOG_DBGF("Failed to map shared memory."); if (close(shm_fd) == -1) LOG_DBGF("Failed to close invalid shm."); - if (shm_unlink(SHM_DU_MAP_FILENAME) == -1) LOG_DBGF("Failed to unlink invalid shm."); - + free(dum); return NULL; } -- cgit v1.2.3 From 502d7b1585edb0ee89cac178f20af8bfdbf5cc64 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 20:39:13 +0200 Subject: lib: shm_ap_rbuff: Fix memleak --- src/lib/shm_ap_rbuff.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/shm_ap_rbuff.c b/src/lib/shm_ap_rbuff.c index 56555533..20bea843 100644 --- a/src/lib/shm_ap_rbuff.c +++ b/src/lib/shm_ap_rbuff.c @@ -173,6 +173,7 @@ struct shm_ap_rbuff * shm_ap_rbuff_open(pid_t api) shm_fd = shm_open(fn, O_RDWR, 0666); if (shm_fd == -1) { LOG_DBG("%d failed opening shared memory %s.", getpid(), fn); + free(rb); return NULL; } -- cgit v1.2.3 From 432e93affce4d5ddcc816f826268bc68d5789756 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 20:51:09 +0200 Subject: tools: cbr: Fix server cleanup The server should cleanup whenever the listen thread exits. --- src/tools/cbr/cbr_server.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/tools/cbr/cbr_server.c b/src/tools/cbr/cbr_server.c index 1890c842..fc23363f 100644 --- a/src/tools/cbr/cbr_server.c +++ b/src/tools/cbr/cbr_server.c @@ -47,18 +47,11 @@ pthread_cond_t fds_signal; void shutdown_server(int signo, siginfo_t * info, void * c) { - int i; - switch(signo) { case SIGINT: case SIGTERM: case SIGHUP: pthread_cancel(listen_thread); - - for (i = 0; i < THREADS_SIZE; i++) { - pthread_cancel(threads[i]); - } - default: return; } @@ -226,19 +219,18 @@ int server_main() exit(EXIT_FAILURE); } - for (i = 0; i < THREADS_SIZE; i++) { - pthread_create(&threads[i], NULL, - worker, NULL); - } + for (i = 0; i < THREADS_SIZE; i++) + pthread_create(&threads[i], NULL, worker, NULL); - pthread_create(&listen_thread, NULL, - listener, NULL); + pthread_create(&listen_thread, NULL, listener, NULL); pthread_join(listen_thread, NULL); - for (i = 0; i < THREADS_SIZE; i++) { + for (i = 0; i < THREADS_SIZE; i++) + pthread_cancel(threads[i]); + + for (i = 0; i < THREADS_SIZE; i++) pthread_join(threads[i], NULL); - } return 0; } -- cgit v1.2.3 From 7e78d3f2fb1d3203aff64e72589cf98649e4fada Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 21:01:28 +0200 Subject: tools: oping: Fix cleanup on exit The server should cleanup whenever the listen thread exits. --- src/tools/oping/oping_server.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/tools/oping/oping_server.c b/src/tools/oping/oping_server.c index eb0b511b..a5021cba 100644 --- a/src/tools/oping/oping_server.c +++ b/src/tools/oping/oping_server.c @@ -37,9 +37,7 @@ void shutdown_server(int signo, siginfo_t * info, void * c) case SIGINT: case SIGTERM: case SIGHUP: - pthread_cancel(server.server_pt); pthread_cancel(server.accept_pt); - pthread_cancel(server.cleaner_pt); default: return; } @@ -154,8 +152,12 @@ int server_main() pthread_create(&server.accept_pt, NULL, accept_thread, NULL); pthread_create(&server.server_pt, NULL, server_thread, NULL); - pthread_join(server.server_pt, NULL); pthread_join(server.accept_pt, NULL); + + pthread_cancel(server.server_pt); + pthread_cancel(server.cleaner_pt); + + pthread_join(server.server_pt, NULL); pthread_join(server.cleaner_pt, NULL); return 0; -- cgit v1.2.3 From 4ca72a0c670082f087529257f620614c7d94c1c9 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 21:20:07 +0200 Subject: lib: sockets: Fix cancellation cleanup When cancelled in read() the fd should be closed and the allocated memory freed. --- src/lib/sockets.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/lib/sockets.c b/src/lib/sockets.c index 9e13b687..5d861cdf 100644 --- a/src/lib/sockets.c +++ b/src/lib/sockets.c @@ -33,6 +33,8 @@ #include #include #include +#include +#include int client_socket_open(char * file_name) { @@ -99,6 +101,11 @@ int server_socket_open(char * file_name) return sockfd; } +void close_ptr(void * o) +{ + close(*(int *) o); +} + irm_msg_t * send_recv_irm_msg(irm_msg_t * msg) { int sockfd; @@ -122,6 +129,9 @@ irm_msg_t * send_recv_irm_msg(irm_msg_t * msg) return NULL; } + pthread_cleanup_push(close_ptr, &sockfd); + pthread_cleanup_push((void (*)(void *)) free, (void *) buf.data); + irm_msg__pack(msg, buf.data); if (write(sockfd, buf.data, buf.len) == -1) { @@ -144,8 +154,9 @@ irm_msg_t * send_recv_irm_msg(irm_msg_t * msg) return NULL; } - free(buf.data); - close(sockfd); + pthread_cleanup_pop(true); + pthread_cleanup_pop(true); + return recv_msg; } -- cgit v1.2.3 From 00761652fb55e16227508457da4bda82b19ef193 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 21:27:45 +0200 Subject: tools: oping: Fix client cleanup --- src/tools/oping/oping_client.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tools/oping/oping_client.c b/src/tools/oping/oping_client.c index 7693ce41..b47eee6c 100644 --- a/src/tools/oping/oping_client.c +++ b/src/tools/oping/oping_client.c @@ -132,12 +132,15 @@ void * writer(void * o) printf("Pinging %s with %d bytes of data:\n\n", client.s_apn, client.size); + pthread_cleanup_push((void (*) (void *)) free, buf); + while (client.sent < client.count) { nanosleep(&wait, NULL); msg->id = htonl(client.sent); if (flow_write(*fdp, buf, client.size) == -1) { printf("Failed to send SDU.\n"); flow_dealloc(*fdp); + free(buf); return (void *) -1; } @@ -148,6 +151,8 @@ void * writer(void * o) pthread_mutex_unlock(&client.lock); } + pthread_cleanup_pop(true); + return (void *) 0; } -- cgit v1.2.3 From c3cf25012053338c0d4fd8fce4e8d94f141603b3 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 22:59:12 +0200 Subject: lib: ipcp.c: Fix memleak upon cancellation --- src/lib/ipcp.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/lib/ipcp.c b/src/lib/ipcp.c index 373baafd..76098691 100644 --- a/src/lib/ipcp.c +++ b/src/lib/ipcp.c @@ -34,9 +34,15 @@ #include #include #include +#include #include #include +static void close_ptr(void * o) +{ + close(*((int *) o)); +} + static ipcp_msg_t * send_recv_ipcp_msg(pid_t api, ipcp_msg_t * msg) { @@ -56,24 +62,26 @@ static ipcp_msg_t * send_recv_ipcp_msg(pid_t api, return NULL; } + free(sock_path); + buf.len = ipcp_msg__get_packed_size(msg); if (buf.len == 0) { close(sockfd); - free(sock_path); return NULL; } buf.data = malloc(IPCP_MSG_BUF_SIZE); if (buf.data == NULL) { close(sockfd); - free(sock_path); return NULL; } + pthread_cleanup_push(close_ptr, (void *) &sockfd); + pthread_cleanup_push((void (*)(void *)) free, (void *) buf.data); + ipcp_msg__pack(msg, buf.data); if (write(sockfd, buf.data, buf.len) == -1) { - free(sock_path); free(buf.data); close(sockfd); return NULL; @@ -81,7 +89,6 @@ static ipcp_msg_t * send_recv_ipcp_msg(pid_t api, count = read(sockfd, buf.data, IPCP_MSG_BUF_SIZE); if (count <= 0) { - free(sock_path); free(buf.data); close(sockfd); return NULL; @@ -89,15 +96,14 @@ static ipcp_msg_t * send_recv_ipcp_msg(pid_t api, recv_msg = ipcp_msg__unpack(NULL, count, buf.data); if (recv_msg == NULL) { - free(sock_path); free(buf.data); close(sockfd); return NULL; } - free(buf.data); - free(sock_path); - close(sockfd); + pthread_cleanup_pop(true); + pthread_cleanup_pop(true); + return recv_msg; } -- cgit v1.2.3 From 93e3e8f3864436d67bba617df5565680c3997542 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Thu, 4 Aug 2016 23:47:35 +0200 Subject: lib: shm_du_map: Fix disabling MULTI_BLOCK --- src/lib/shm_du_map.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/shm_du_map.c b/src/lib/shm_du_map.c index d88e5c84..6527e035 100644 --- a/src/lib/shm_du_map.c +++ b/src/lib/shm_du_map.c @@ -412,8 +412,8 @@ ssize_t shm_du_map_write(struct shm_du_map * dum, #ifdef SHM_DU_MAP_MULTI_BLOCK long blocks = 0; long padblocks = 0; - int sz = size + sizeof *sdb; #endif + int sz = size + sizeof *sdb; uint8_t * write_pos; ssize_t idx = -1; @@ -497,8 +497,8 @@ ssize_t shm_du_map_write_b(struct shm_du_map * dum, #ifdef SHM_DU_MAP_MULTI_BLOCK long blocks = 0; long padblocks = 0; - int sz = size + sizeof *sdb; #endif + int sz = size + sizeof *sdb; uint8_t * write_pos; ssize_t idx = -1; -- cgit v1.2.3 From 55356470f9fe0b60bd9e0db1e90dbca6b4e7db63 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 5 Aug 2016 15:04:50 +0200 Subject: lib:shm_du_map: Fix wrong element access The check whether the du map is empty should be first. --- src/lib/shm_du_map.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib/shm_du_map.c b/src/lib/shm_du_map.c index 6527e035..8d197232 100644 --- a/src/lib/shm_du_map.c +++ b/src/lib/shm_du_map.c @@ -91,13 +91,14 @@ static void garbage_collect(struct shm_du_map * dum) { #ifdef SHM_DU_MAP_MULTI_BLOCK struct shm_du_buff * sdb; - while ((sdb = get_tail_ptr(dum))->dst_api == -1 && - !shm_map_empty(dum)) + while (!shm_map_empty(dum) && + (sdb = get_tail_ptr(dum))->dst_api == -1) + *dum->ptr_tail = (*dum->ptr_tail + sdb->blocks) & (SHM_BUFFER_SIZE - 1); #else - while (get_tail_ptr(dum)->dst_api == -1 && - !shm_map_empty(dum)) + while (!shm_map_empty(dum) && + get_tail_ptr(dum)->dst_api == -1) *dum->ptr_tail = (*dum->ptr_tail + 1) & (SHM_BUFFER_SIZE - 1); -- cgit v1.2.3 From 84d540904e13882e8c299f98e5931a1213b6d47e Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 5 Aug 2016 15:06:37 +0200 Subject: ipcpd: Fix memleak The entry from the ringbuffer was never freed, causing memleak per SDU. --- src/ipcpd/local/main.c | 2 ++ src/ipcpd/shim-eth-llc/main.c | 2 ++ src/ipcpd/shim-udp/main.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/ipcpd/local/main.c b/src/ipcpd/local/main.c index e77a0403..99580a45 100644 --- a/src/ipcpd/local/main.c +++ b/src/ipcpd/local/main.c @@ -210,6 +210,8 @@ static void * ipcp_local_sdu_loop(void * o) pthread_rwlock_unlock(&_ap_instance->flows_lock); pthread_rwlock_unlock(&_ipcp->state_lock); + + free(e); } return (void *) 1; diff --git a/src/ipcpd/shim-eth-llc/main.c b/src/ipcpd/shim-eth-llc/main.c index 3b70b955..9f6573c9 100644 --- a/src/ipcpd/shim-eth-llc/main.c +++ b/src/ipcpd/shim-eth-llc/main.c @@ -846,6 +846,8 @@ static void * eth_llc_ipcp_sdu_writer(void * o) shm_du_map_remove(shim_data(_ipcp)->dum, e->index); pthread_rwlock_unlock(&_ipcp->state_lock); + + free(e); } return (void *) 1; diff --git a/src/ipcpd/shim-udp/main.c b/src/ipcpd/shim-udp/main.c index 49fd7772..9d6d15a7 100644 --- a/src/ipcpd/shim-udp/main.c +++ b/src/ipcpd/shim-udp/main.c @@ -802,6 +802,8 @@ static void * ipcp_udp_sdu_loop(void * o) shm_du_map_remove(_ap_instance->dum, e->index); pthread_rwlock_unlock(&_ipcp->state_lock); + + free(e); } return (void *) 1; -- cgit v1.2.3 From cb8ad2d29475a907b4f662aa86c18b45ff5a2c0a Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 5 Aug 2016 15:17:51 +0200 Subject: irmd: registry: Fix double free The req_ae_name is freed when the unpacked message is cleaned up. --- src/irmd/registry.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/irmd/registry.c b/src/irmd/registry.c index fc7213ab..7745efaa 100644 --- a/src/irmd/registry.c +++ b/src/irmd/registry.c @@ -253,9 +253,6 @@ static void reg_entry_destroy(struct reg_entry * e) if (e->name != NULL) free(e->name); - if (e->req_ae_name != NULL) - free(e->req_ae_name); - list_for_each_safe(pos, n, &e->reg_apis) { struct reg_api * i = list_entry(pos, struct reg_api, next); reg_api_destroy(i); -- cgit v1.2.3 From d2cd2e05bb6e7e92fc6fadaa318ec37c8b3e062b Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sat, 6 Aug 2016 02:45:17 +0200 Subject: lib: shm_du_map: Fix wrong padding Padding was miscalculated, causing lockups in the fast path. --- src/lib/shm_du_map.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/shm_du_map.c b/src/lib/shm_du_map.c index 8d197232..3e2c99d4 100644 --- a/src/lib/shm_du_map.c +++ b/src/lib/shm_du_map.c @@ -332,7 +332,7 @@ void * shm_du_map_sanitize(void * o) } if (ret == ETIMEDOUT) { - LOG_DBGF("SDU timed out."); + LOG_DBGF("SDU timed out (dst: %d).", api); clean_sdus(dum, api, false); } } @@ -439,7 +439,7 @@ ssize_t shm_du_map_write(struct shm_du_map * dum, ++blocks; } - if (blocks + *dum->ptr_head > SHM_BUFFER_SIZE - 1) + if (blocks + *dum->ptr_head > SHM_BUFFER_SIZE) padblocks = SHM_BUFFER_SIZE - *dum->ptr_head; if (!shm_map_free(dum, (blocks + padblocks))) { @@ -528,7 +528,7 @@ ssize_t shm_du_map_write_b(struct shm_du_map * dum, ++blocks; } - if (blocks + *dum->ptr_head > SHM_BUFFER_SIZE - 1) + if (blocks + *dum->ptr_head > SHM_BUFFER_SIZE) padblocks = SHM_BUFFER_SIZE - *dum->ptr_head; while (!shm_map_free(dum, (blocks + padblocks))) { -- cgit v1.2.3 From 656fb082a3ab596555ba661ee98ab8400f9dd4f1 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sat, 6 Aug 2016 02:46:15 +0200 Subject: tools: oping: Fix wrong delay calculation --- src/tools/oping/oping_client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/oping/oping_client.c b/src/tools/oping/oping_client.c index b47eee6c..0d4a10af 100644 --- a/src/tools/oping/oping_client.c +++ b/src/tools/oping/oping_client.c @@ -115,7 +115,8 @@ void * writer(void * o) { int * fdp = (int *) o; struct timespec now; - struct timespec wait = {client.interval / 1000, client.interval % 1000}; + struct timespec wait = {client.interval / 1000, + (client.interval % 1000) * MILLION}; struct oping_msg * msg; char * buf = malloc(client.size); -- cgit v1.2.3 From 8450af0e2950e5a1415e38702d6169cc8a732294 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sat, 6 Aug 2016 12:51:26 +0200 Subject: lib: shm_ap_rbuff: Remove obsolete clean function --- src/lib/shm_ap_rbuff.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/lib/shm_ap_rbuff.c b/src/lib/shm_ap_rbuff.c index 20bea843..e9c51533 100644 --- a/src/lib/shm_ap_rbuff.c +++ b/src/lib/shm_ap_rbuff.c @@ -52,9 +52,6 @@ #define shm_rbuff_empty(rb) (*rb->ptr_head == *rb->ptr_tail) #define head_el_ptr(rb) (rb->shm_base + *rb->ptr_head) #define tail_el_ptr(rb) (rb->shm_base + *rb->ptr_tail) -#define clean_sdus(rb) \ - while (!shm_rbuff_empty(rb) && tail_el_ptr(rb)->port_id < 0) \ - *rb->ptr_tail = (*rb->ptr_tail + 1) & (SHM_BUFFER_SIZE -1); \ struct shm_ap_rbuff { struct rb_entry * shm_base; /* start of entry */ @@ -316,8 +313,6 @@ int shm_ap_rbuff_peek(struct shm_ap_rbuff * rb, pthread_mutex_consistent(rb->lock); } - clean_sdus(rb); - while (shm_rbuff_empty(rb)) { if (timeout != NULL) ret = pthread_cond_timedwait(rb->add, @@ -326,8 +321,6 @@ int shm_ap_rbuff_peek(struct shm_ap_rbuff * rb, else ret = pthread_cond_wait(rb->add, rb->lock); - clean_sdus(rb); - if (ret == EOWNERDEAD) { LOG_DBG("Recovering dead mutex."); pthread_mutex_consistent(rb->lock); @@ -361,8 +354,6 @@ struct rb_entry * shm_ap_rbuff_read(struct shm_ap_rbuff * rb) pthread_mutex_consistent(rb->lock); } - clean_sdus(rb); - while (shm_rbuff_empty(rb)) if (pthread_cond_wait(rb->add, rb->lock) == EOWNERDEAD) { LOG_DBG("Recovering dead mutex."); @@ -393,8 +384,6 @@ ssize_t shm_ap_rbuff_read_port(struct shm_ap_rbuff * rb, int port_id) pthread_mutex_consistent(rb->lock); } - clean_sdus(rb); - if (shm_rbuff_empty(rb) || tail_el_ptr(rb)->port_id != port_id) { pthread_mutex_unlock(rb->lock); return -1; @@ -429,8 +418,6 @@ ssize_t shm_ap_rbuff_read_port_b(struct shm_ap_rbuff * rb, ts_add(&abstime, timeout, &abstime); } - clean_sdus(rb); - pthread_cleanup_push((void(*)(void *))pthread_mutex_unlock, (void *) rb->lock); @@ -442,8 +429,6 @@ ssize_t shm_ap_rbuff_read_port_b(struct shm_ap_rbuff * rb, else ret = pthread_cond_wait(rb->del, rb->lock); - clean_sdus(rb); - if (ret == EOWNERDEAD) { LOG_DBG("Recovering dead mutex."); pthread_mutex_consistent(rb->lock); -- cgit v1.2.3