From 6166259f74855fe9e6976175eeb7b16912bc477a Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Tue, 27 Dec 2016 23:30:06 +0100 Subject: lib: Fix memleak in irm.c --- src/lib/irm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/irm.c b/src/lib/irm.c index 4b9a6285..0d9d0f86 100644 --- a/src/lib/irm.c +++ b/src/lib/irm.c @@ -358,6 +358,9 @@ int irm_bind_ap(char * ap, msg.opts = opts; recv_msg = send_recv_irm_msg(&msg); + + free(full_ap_name); + if (recv_msg == NULL) return -1; @@ -369,7 +372,6 @@ int irm_bind_ap(char * ap, ret = recv_msg->result; irm_msg__free_unpacked(recv_msg, NULL); - free(full_ap_name); return ret; } -- cgit v1.2.3 From 314c5323d3a038394f43e84d72594b78d17b1a46 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 00:36:36 +0100 Subject: include, lib: Report communication failure with the IRMd An errno EIRMD has been added to errno.h to indicate failure to send a message tot the IRMd. The IRM tool will report such errors, which makes it easier for users to detect that the IRM was not started or has failed. --- include/ouroboros/errno.h | 1 + src/lib/dev.c | 42 +++++++++++------------------------------- src/lib/irm.c | 25 ++++++++++++------------- src/tools/irm/irm.c | 22 ++++++++++++++++++---- src/tools/irm/irm_bind_ap.c | 4 ++-- 5 files changed, 44 insertions(+), 50 deletions(-) (limited to 'src/lib') diff --git a/include/ouroboros/errno.h b/include/ouroboros/errno.h index 0b55c00d..e50b180d 100644 --- a/include/ouroboros/errno.h +++ b/include/ouroboros/errno.h @@ -27,5 +27,6 @@ #define ENOTALLOC 1000 /* Flow is not allocated */ #define EIPCPTYPE 1001 /* Unknown IPCP type */ +#define EIRMD 1002 /* Failed to contact IRMD */ #endif diff --git a/src/lib/dev.c b/src/lib/dev.c index 8eb98506..3a1df16d 100644 --- a/src/lib/dev.c +++ b/src/lib/dev.c @@ -176,7 +176,7 @@ static int api_announce(char * ap_name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) { - return -1; + return -EIRMD; } if (!recv_msg->has_result || (ret = recv_msg->result)) { @@ -359,7 +359,7 @@ int flow_accept(char ** ae_name, qosspec_t * qos) recv_msg = send_recv_irm_msg_b(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_api || !recv_msg->has_port_id) { irm_msg__free_unpacked(recv_msg, NULL); @@ -465,7 +465,7 @@ int flow_alloc_resp(int fd, int response) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); @@ -476,16 +476,6 @@ int flow_alloc_resp(int fd, int response) irm_msg__free_unpacked(recv_msg, NULL); - if (response) { - pthread_rwlock_rdlock(&ai.data_lock); - pthread_rwlock_wrlock(&ai.flows_lock); - - reset_flow(fd); - - pthread_rwlock_unlock(&ai.flows_lock); - pthread_rwlock_unlock(&ai.data_lock); - } - return ret; } @@ -517,7 +507,7 @@ int flow_alloc(char * dst_name, char * src_ae_name, qosspec_t * qos) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_api || !recv_msg->has_port_id) { irm_msg__free_unpacked(recv_msg, NULL); @@ -603,27 +593,17 @@ int flow_alloc_res(int fd) recv_msg = send_recv_irm_msg_b(&msg); if (recv_msg == NULL) - result = -1; + return -EIRMD; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); - result = -1; + return -1; } result = recv_msg->result; irm_msg__free_unpacked(recv_msg, NULL); - if (result) { - pthread_rwlock_rdlock(&ai.data_lock); - pthread_rwlock_wrlock(&ai.flows_lock); - - reset_flow(fd); - - pthread_rwlock_unlock(&ai.flows_lock); - pthread_rwlock_unlock(&ai.data_lock); - } - return result; } @@ -655,9 +635,9 @@ int flow_dealloc(int fd) pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); - recv_msg = send_recv_irm_msg_b(&msg); + recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_result) { irm_msg__free_unpacked(recv_msg, NULL); @@ -1208,7 +1188,7 @@ int ipcp_create_r(pid_t api) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -1254,7 +1234,7 @@ int ipcp_flow_req_arr(pid_t api, char * dst_name, char * src_ae_name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (!recv_msg->has_port_id || !recv_msg->has_api) { irm_msg__free_unpacked(recv_msg, NULL); @@ -1336,7 +1316,7 @@ int ipcp_flow_alloc_reply(int fd, int response) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); diff --git a/src/lib/irm.c b/src/lib/irm.c index 0d9d0f86..969a78c3 100644 --- a/src/lib/irm.c +++ b/src/lib/irm.c @@ -47,7 +47,7 @@ pid_t irm_create_ipcp(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -75,7 +75,7 @@ int irm_destroy_ipcp(pid_t api) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -147,9 +147,8 @@ int irm_bootstrap_ipcp(pid_t api, } recv_msg = send_recv_irm_msg(&msg); - if (recv_msg == NULL) { - return -1; - } + if (recv_msg == NULL) + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -179,7 +178,7 @@ ssize_t irm_list_ipcps(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) { free(msg.dif_name); - return -1; + return -EIRMD; } if (recv_msg->apis == NULL) { @@ -227,7 +226,7 @@ int irm_enroll_ipcp(pid_t api, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) { free(msg.dif_name); - return -1; + return -EIRMD; } if (recv_msg->has_result == false) { @@ -362,7 +361,7 @@ int irm_bind_ap(char * ap, free(full_ap_name); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -391,7 +390,7 @@ int irm_bind_api(pid_t api, char * name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -419,7 +418,7 @@ int irm_unbind_ap(char * ap, char * name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -448,7 +447,7 @@ int irm_unbind_api(pid_t api, char * name) recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -481,7 +480,7 @@ int irm_reg(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); @@ -515,7 +514,7 @@ int irm_unreg(char * name, recv_msg = send_recv_irm_msg(&msg); if (recv_msg == NULL) - return -1; + return -EIRMD; if (recv_msg->has_result == false) { irm_msg__free_unpacked(recv_msg, NULL); diff --git a/src/tools/irm/irm.c b/src/tools/irm/irm.c index 7b2f3331..ce92d3d5 100644 --- a/src/tools/irm/irm.c +++ b/src/tools/irm/irm.c @@ -20,12 +20,15 @@ */ #include -#include -#include +#include #include "irm_ops.h" #include "irm_utils.h" +#include +#include +#include + static void usage(void) { printf("Usage: irm [OPERATION]\n\n" @@ -71,12 +74,23 @@ static int do_cmd(const char * argv0, return -1; } -int main(int argc, char ** argv) { +int main(int argc, char ** argv) +{ + int ret = 0; if (argc < 2) { usage(); return -1; } - return do_cmd(argv[1], argc - 1, argv + 1); + ret = do_cmd(argv[1], argc - 1, argv + 1); + + if (ret == -EIRMD) + printf("Failed to communicate with the " + "Ouroboros IPC Resource Manager daemon.\n"); + + if (ret) + exit(EXIT_FAILURE); + + exit(EXIT_SUCCESS); } diff --git a/src/tools/irm/irm_bind_ap.c b/src/tools/irm/irm_bind_ap.c index e8e5eb26..a5be5a03 100644 --- a/src/tools/irm/irm_bind_ap.c +++ b/src/tools/irm/irm_bind_ap.c @@ -90,13 +90,13 @@ int do_bind_ap(int argc, char ** argv) ret = irm_bind_ap(ap_name, name, flags, argc, argv); if (ret == -ENOENT) { printf("%s does not exist.\n", ap_name); - return -1; + return ret; } if (ret == -EPERM) { printf("Cannot execute %s, please check permissions.\n", ap_name); - return -1; + return ret; } if (temp != NULL) -- cgit v1.2.3 From 6d14473d4fd0629125d9a96d9292deb32ba7e0a8 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 00:41:31 +0100 Subject: lib: Fix leaks in cdap Fixes a memleak of cdap_req structs and correcly releases invoke_ids if the cdap_request_send operation fails during its execution. --- src/lib/cdap.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'src/lib') diff --git a/src/lib/cdap.c b/src/lib/cdap.c index 5671c515..17104770 100644 --- a/src/lib/cdap.c +++ b/src/lib/cdap.c @@ -105,7 +105,8 @@ static int release_invoke_id(struct cdap * instance, int id) #define cdap_sent_has_key(i, key) (cdap_sent_get_by_key(i, key) != NULL) -struct cdap_req * cdap_sent_get_by_key(struct cdap * instance, cdap_key_t key) +static struct cdap_req * cdap_sent_get_by_key(struct cdap * instance, + cdap_key_t key) { struct list_head * p = NULL; struct cdap_req * req = NULL; @@ -128,13 +129,17 @@ struct cdap_req * cdap_sent_get_by_key(struct cdap * instance, cdap_key_t key) return NULL; } -static int cdap_sent_add(struct cdap * instance, struct cdap_req * req) +static struct cdap_req * cdap_sent_add(struct cdap * instance, cdap_key_t key) { - assert (instance); - assert (req); + struct cdap_req * req; - if (cdap_sent_has_key(instance, req->key)) - return -EPERM; + assert(instance); + assert(key >= 0); + assert(!cdap_sent_has_key(instance, key)); + + req = cdap_req_create(key); + if (req == NULL) + return NULL; pthread_rwlock_wrlock(&instance->sent_lock); @@ -142,7 +147,7 @@ static int cdap_sent_add(struct cdap * instance, struct cdap_req * req) pthread_rwlock_unlock(&instance->sent_lock); - return 0; + return req; } static void cdap_sent_del(struct cdap * instance, struct cdap_req * req) @@ -157,6 +162,8 @@ static void cdap_sent_del(struct cdap * instance, struct cdap_req * req) list_del(&req->next); pthread_rwlock_unlock(&instance->sent_lock); + + cdap_req_destroy(req); } static void cdap_sent_destroy(struct cdap * instance) @@ -249,6 +256,7 @@ static void * sdu_reader(void * o) free(rcvd); continue; } + rcvd->iid = msg->invoke_id; rcvd->flags = msg->flags; rcvd->name = strdup(msg->name); @@ -494,18 +502,15 @@ cdap_key_t cdap_request_send(struct cdap * instance, key = invoke_id_to_key(iid); - req = cdap_req_create(key); - if (req == NULL) - return INVALID_CDAP_KEY; - - if (cdap_sent_add(instance, req)) { - cdap_req_destroy(req); + req = cdap_sent_add(instance, key); + if (req == NULL) { + release_invoke_id(instance, iid); return INVALID_CDAP_KEY; } if (write_msg(instance, &msg)) { cdap_sent_del(instance, req); - cdap_req_destroy(req); + release_invoke_id(instance, iid); return INVALID_CDAP_KEY; } -- cgit v1.2.3 From 502e7bb39096f6ce7718c29d9616bbd0314d045e Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Wed, 28 Dec 2016 09:05:44 +0100 Subject: lib: Fix race in destruction of cdap_req If cdap_req_destroy was called while in REQ_DONE, cdap->state would be accessed in cdap_req_respond. --- src/lib/cdap_req.c | 25 ++++++++++++++++--------- src/lib/cdap_req.h | 3 ++- 2 files changed, 18 insertions(+), 10 deletions(-) (limited to 'src/lib') diff --git a/src/lib/cdap_req.c b/src/lib/cdap_req.c index f80b10d7..57ad22c5 100644 --- a/src/lib/cdap_req.c +++ b/src/lib/cdap_req.c @@ -70,14 +70,14 @@ void cdap_req_destroy(struct cdap_req * creq) } if (creq->state == REQ_INIT) - creq->state = REQ_DONE; + creq->state = REQ_NULL; if (creq->state == REQ_PENDING) { creq->state = REQ_DESTROY; pthread_cond_broadcast(&creq->cond); } - while (creq->state != REQ_DONE) + while (creq->state != REQ_NULL) pthread_cond_wait(&creq->cond, &creq->lock); pthread_mutex_unlock(&creq->lock); @@ -109,17 +109,21 @@ int cdap_req_wait(struct cdap_req * creq) creq->state = REQ_PENDING; while (creq->state == REQ_PENDING) { - if ((ret = -pthread_cond_timedwait(&creq->cond, - &creq->lock, - &abstime)) == -ETIMEDOUT) + ret = -pthread_cond_timedwait(&creq->cond, + &creq->lock, + &abstime); + if (ret == -ETIMEDOUT) break; } - if (creq->state == REQ_DESTROY) + if (creq->state == REQ_DESTROY) { ret = -1; - - creq->state = REQ_DONE; - pthread_cond_broadcast(&creq->cond); + creq->state = REQ_NULL; + pthread_cond_broadcast(&creq->cond); + } else { + creq->state = REQ_DONE; + pthread_cond_broadcast(&creq->cond); + } pthread_mutex_unlock(&creq->lock); @@ -146,5 +150,8 @@ void cdap_req_respond(struct cdap_req * creq, int response, buffer_t data) while (creq->state == REQ_RESPONSE) pthread_cond_wait(&creq->cond, &creq->lock); + creq->state = REQ_NULL; + pthread_cond_broadcast(&creq->cond); + pthread_mutex_unlock(&creq->lock); } diff --git a/src/lib/cdap_req.h b/src/lib/cdap_req.h index 9d5cb0c8..b2ded060 100644 --- a/src/lib/cdap_req.h +++ b/src/lib/cdap_req.h @@ -31,7 +31,8 @@ #include enum creq_state { - REQ_INIT = 0, + REQ_NULL = 0, + REQ_INIT, REQ_PENDING, REQ_RESPONSE, REQ_DONE, -- cgit v1.2.3