From a64d33fb44f30196711108311896cdbe711cc4d3 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sat, 14 May 2016 23:11:04 +0200 Subject: ipcpd: shim-udp: fixed locking. The fd_to_port_id function has no internal locking and must now be called under lock. --- src/ipcpd/shim-udp/main.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/ipcpd/shim-udp/main.c b/src/ipcpd/shim-udp/main.c index 917c343b..2d1691aa 100644 --- a/src/ipcpd/shim-udp/main.c +++ b/src/ipcpd/shim-udp/main.c @@ -189,24 +189,19 @@ void shim_ap_fini() free(_ap_instance); } +/* only call this under flows_lock */ static int port_id_to_fd(int port_id) { int i; - rw_lock_rdlock(&_ap_instance->flows_lock); - for (i = 0; i < AP_MAX_FLOWS; ++i) { if (_ap_instance->flows[i].port_id == port_id && _ap_instance->flows[i].state != FLOW_NULL) { - rw_lock_unlock(&_ap_instance->flows_lock); - return i; } } - rw_lock_unlock(&_ap_instance->flows_lock); - return -1; } @@ -275,7 +270,7 @@ struct ipcp_udp_data * ipcp_udp_data_create() udp_data = malloc(sizeof *udp_data); if (udp_data == NULL) { - LOG_DBGF("Failed to allocate."); + LOG_ERR("Failed to allocate."); return NULL; } @@ -513,8 +508,12 @@ static void * ipcp_udp_sdu_loop(void * o) continue; } + rw_lock_rdlock(&_ap_instance->flows_lock); + fd = port_id_to_fd(e->port_id); + rw_lock_unlock(&_ap_instance->flows_lock); + if (fd == -1) { rw_lock_unlock(&_ap_instance->data_lock); free(e); @@ -1054,11 +1053,7 @@ static int ipcp_udp_flow_alloc_resp(int port_id, int response) { struct shm_ap_rbuff * rb; - int fd = port_id_to_fd(port_id); - if (fd < 0) { - LOG_DBGF("Could not find flow with port_id %d.", port_id); - return 0; - } + int fd = -1; if (response) return 0; @@ -1067,6 +1062,14 @@ static int ipcp_udp_flow_alloc_resp(int port_id, rw_lock_rdlock(&_ap_instance->flows_lock); + fd = port_id_to_fd(port_id); + if (fd < 0) { + rw_lock_unlock(&_ap_instance->flows_lock); + + LOG_DBGF("Could not find flow with port_id %d.", port_id); + return 0; + } + if (_ap_instance->flows[fd].state != FLOW_PENDING) { rw_lock_unlock(&_ap_instance->flows_lock); @@ -1110,16 +1113,19 @@ static int ipcp_udp_flow_alloc_resp(int port_id, static int ipcp_udp_flow_dealloc(int port_id) { - int fd = port_id_to_fd(port_id); + int fd = -1; struct shm_ap_rbuff * rb; + rw_lock_wrlock(&_ap_instance->flows_lock); + + fd = port_id_to_fd(port_id); if (fd < 0) { + rw_lock_unlock(&_ap_instance->flows_lock); + LOG_DBGF("Could not find flow with port_id %d.", port_id); return 0; } - rw_lock_wrlock(&_ap_instance->flows_lock); - _ap_instance->flows[fd].state = FLOW_NULL; _ap_instance->flows[fd].port_id = 0; rb = _ap_instance->flows[fd].rb; -- cgit v1.2.3 From cbd6ccb7fa372832715b0e143c816f569bd07ccd Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sun, 15 May 2016 02:26:27 +0200 Subject: lib: irmd: fixed flow deallocation Port_id's weren't correctly released. Also removes remaining debug logs from the library. --- src/irmd/main.c | 2 ++ src/lib/dev.c | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/irmd/main.c b/src/irmd/main.c index 65c173de..9c515d2a 100644 --- a/src/irmd/main.c +++ b/src/irmd/main.c @@ -1059,6 +1059,8 @@ static int flow_dealloc(int port_id) list_del(&e->next); + bmp_release(instance->port_ids, port_id); + pthread_mutex_unlock(&instance->r_lock); free(e); diff --git a/src/lib/dev.c b/src/lib/dev.c index 440f40f9..4b5e56be 100644 --- a/src/lib/dev.c +++ b/src/lib/dev.c @@ -163,10 +163,8 @@ int ap_reg(char ** difs, return -EINVAL; } - if (_ap_instance == NULL) { - LOG_DBG("ap_init was not called."); - return -1; - } + if (_ap_instance == NULL) + return -1; /* -ENOTINIT */ msg.code = IRM_MSG_CODE__IRM_AP_REG; msg.has_pid = true; @@ -468,11 +466,16 @@ int flow_dealloc(int fd) msg.code = IRM_MSG_CODE__IRM_FLOW_DEALLOC; msg.has_port_id = true; - - rw_lock_rdlock(&_ap_instance->data_lock); + rw_lock_rdlock(&_ap_instance->flows_lock); msg.port_id = _ap_instance->flows[fd].port_id; + rw_lock_unlock(&_ap_instance->flows_lock); + + rw_lock_wrlock(&_ap_instance->data_lock); + + bmp_release(_ap_instance->fds, fd); + rw_lock_unlock(&_ap_instance->data_lock); recv_msg = send_recv_irm_msg(&msg); @@ -556,7 +559,7 @@ ssize_t flow_write(int fd, void * buf, size_t count) return 0; } else { while (shm_ap_rbuff_write(_ap_instance->flows[fd].rb, &e) < 0) - LOG_DBGF("Couldn't write to rbuff."); + ; } rw_lock_unlock(&_ap_instance->data_lock); -- cgit v1.2.3 From e79f151adc07538fbb1057442df2c1fe6ac01836 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sun, 15 May 2016 02:31:54 +0200 Subject: lib: shm_ap_rbuff: simplified file offsets Used simpler pointer arithmetic to calculate the file offset pointers. --- src/lib/shm_ap_rbuff.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/lib/shm_ap_rbuff.c b/src/lib/shm_ap_rbuff.c index da6f0e33..0524a96c 100644 --- a/src/lib/shm_ap_rbuff.c +++ b/src/lib/shm_ap_rbuff.c @@ -38,7 +38,7 @@ #define SHM_RBUFF_FILE_SIZE (SHM_RBUFF_SIZE * sizeof(struct rb_entry) \ + 2 * sizeof(size_t) + sizeof(pthread_mutex_t)) -#define shm_rbuff_used(rb)((*rb->ptr_head + SHM_RBUFF_SIZE - *rb->ptr_tail)\ +#define shm_rbuff_used(rb)((*rb->ptr_head + SHM_RBUFF_SIZE - *rb->ptr_tail) \ & (SHM_RBUFF_SIZE - 1)) #define shm_rbuff_free(rb)(shm_rbuff_used(rb) + 1 < SHM_RBUFF_SIZE) @@ -105,12 +105,10 @@ struct shm_ap_rbuff * shm_ap_rbuff_create() return NULL; } - rb->shm_base = shm_base; - rb->ptr_head = (size_t *) (rb->shm_base + SHM_RBUFF_SIZE); - rb->ptr_tail = (size_t *) - ((uint8_t *) rb->ptr_head + sizeof(size_t)); - rb->shm_mutex = (pthread_mutex_t *) - ((uint8_t *) rb->ptr_tail + sizeof(size_t)); + rb->shm_base = shm_base; + rb->ptr_head = (size_t *) (rb->shm_base + SHM_RBUFF_SIZE); + rb->ptr_tail = rb->ptr_head + 1; + rb->shm_mutex = (pthread_mutex_t *) (rb->ptr_tail + 1); pthread_mutexattr_init(&attr); pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED); @@ -165,12 +163,10 @@ struct shm_ap_rbuff * shm_ap_rbuff_open(pid_t pid) return NULL; } - rb->shm_base = shm_base; - rb->ptr_head = (size_t *) (rb->shm_base + SHM_RBUFF_SIZE); - rb->ptr_tail = (size_t *) - ((uint8_t *) rb->ptr_head + sizeof(size_t)); - rb->shm_mutex = (pthread_mutex_t *) - ((uint8_t *) rb->ptr_tail + sizeof(size_t)); + rb->shm_base = shm_base; + rb->ptr_head = (size_t *) (rb->shm_base + SHM_RBUFF_SIZE); + rb->ptr_tail = rb->ptr_head + 1; + rb->shm_mutex = (pthread_mutex_t *) (rb->ptr_tail + 1); rb->fd = shm_fd; rb->pid = pid; -- cgit v1.2.3