[sr-dev] git:master: dmq: Fixed bug/ error in original code where sip_msg was parsed after cloning to shm, leading to memory errors. Also fixed several memory leaks.

Module: sip-router Branch: master Commit: 3b028d308fac3a4f7ae1e74021882657753f2ee8 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=3b028d3… Author: Charles Chance &lt;charles.chance(a)sipcentric.com&gt; Committer: Charles Chance &lt;charles.chance(a)sipcentric.com&gt; Date: Wed Oct 23 12:01:27 2013 +0100 dmq: Fixed bug/error in original code where sip_msg was parsed after cloning to shm, leading to memory errors. Also fixed several memory leaks. --- modules/dmq/dmqnode.c | 22 ++++++++++------------ modules/dmq/message.c | 10 +--------- modules/dmq/notification_peer.c | 5 ----- modules/dmq/worker.c | 34 ++++++++++++++++++++++++++++------ modules/dmq/worker.h | 1 + 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/modules/dmq/dmqnode.c b/modules/dmq/dmqnode.c index 5bf0fc3..bc7c108 100644 --- a/modules/dmq/dmqnode.c +++ b/modules/dmq/dmqnode.c @@ -203,12 +203,7 @@ dmq_node_t* build_dmq_node(str* uri, int shm) { error: if(ret!=NULL) { - /* tbd: free uri and params */ - if(shm) { - shm_free(ret); - } else { - pkg_free(ret); - } + destroy_dmq_node(ret, shm); } return NULL; } @@ -232,7 +227,6 @@ dmq_node_t* find_dmq_node_uri(dmq_node_list_t* list, str* uri) */ void destroy_dmq_node(dmq_node_t* node, int shm) { - /* tbd: check inner fields */ if(shm) { shm_free_node(node); } else { @@ -278,9 +272,7 @@ dmq_node_t* shm_dup_node(dmq_node_t* node) } return newnode; error: - if(newnode->orig_uri.s!=NULL) - shm_free(newnode->orig_uri.s); - shm_free(newnode); + destroy_dmq_node(newnode, 1); return NULL; } @@ -289,7 +281,10 @@ error: */ void shm_free_node(dmq_node_t* node) { - shm_free(node->orig_uri.s); + if (node->orig_uri.s!=NULL) + shm_free(node->orig_uri.s); + if (node->params!=NULL) + shm_free_params(node->params); shm_free(node); } @@ -298,7 +293,10 @@ void shm_free_node(dmq_node_t* node) */ void pkg_free_node(dmq_node_t* node) { - pkg_free(node->orig_uri.s); + if (node->orig_uri.s!=NULL) + pkg_free(node->orig_uri.s); + if (node->params!=NULL) + free_params(node->params); pkg_free(node); } diff --git a/modules/dmq/message.c b/modules/dmq/message.c index 05b77f1..a695fee 100644 --- a/modules/dmq/message.c +++ b/modules/dmq/message.c @@ -26,7 +26,6 @@ #include "../../parser/parse_to.h" #include "../../parser/parse_uri.h" -#include "../../sip_msg_clone.h" #include "../../parser/parse_content.h" #include "../../parser/parse_from.h" #include "../../ut.h" @@ -46,8 +45,6 @@ str dmq_404_rpl = str_init("User Not Found"); int dmq_handle_message(struct sip_msg* msg, char* str1, char* str2) { dmq_peer_t* peer; - struct sip_msg* cloned_msg = NULL; - int cloned_msg_len; if ((parse_sip_msg_uri(msg) < 0) || (!msg->parsed_uri.user.s)) { LM_ERR("error parsing msg uri\n"); goto error; @@ -68,12 +65,7 @@ int dmq_handle_message(struct sip_msg* msg, char* str1, char* str2) return 0; } LM_DBG("dmq_handle_message peer found: %.*s\n", msg->parsed_uri.user.len, msg->parsed_uri.user.s); - cloned_msg = sip_msg_shm_clone(msg, &cloned_msg_len, 1); - if(!cloned_msg) { - LM_ERR("error cloning sip message\n"); - goto error; - } - if(add_dmq_job(cloned_msg, peer)<0) { + if(add_dmq_job(msg, peer)<0) { LM_ERR("failed to add dmq job\n"); goto error; } diff --git a/modules/dmq/notification_peer.c b/modules/dmq/notification_peer.c index eb48ee8..fc35618 100644 --- a/modules/dmq/notification_peer.c +++ b/modules/dmq/notification_peer.c @@ -162,11 +162,6 @@ int dmq_notification_callback(struct sip_msg* msg, peer_reponse_t* resp) unsigned int maxforwards = 1; /* received dmqnode list */ LM_DBG("dmq triggered from dmq_notification_callback\n"); - /* parse the message headers */ - if(parse_headers(msg, HDR_EOH_F, 0) < 0) { - LM_ERR("error parsing message headers\n"); - goto error; - } /* extract the maxforwards value, if any */ if(msg->maxforwards) { diff --git a/modules/dmq/worker.c b/modules/dmq/worker.c index a265712..3965f5a 100644 --- a/modules/dmq/worker.c +++ b/modules/dmq/worker.c @@ -28,6 +28,7 @@ #include "worker.h" #include "../../data_lump_rpl.h" #include "../../mod_fix.h" +#include "../../sip_msg_clone.h" /** * @brief set the body of a response @@ -131,17 +132,31 @@ void worker_loop(int id) int add_dmq_job(struct sip_msg* msg, dmq_peer_t* peer) { int i, found_available = 0; - int ret; dmq_job_t new_job = { 0 }; dmq_worker_t* worker; + struct sip_msg* cloned_msg = NULL; + int cloned_msg_len; + + /* Pre-parse headers so they are included in our clone. Parsing later + * will result in linking pkg structures to shm msg, eventually leading + * to memory errors. */ + if (parse_headers(msg, HDR_EOH_F, 0) == -1) { + LM_ERR("failed to parse headers\n"); + return -1; + } + + cloned_msg = sip_msg_shm_clone(msg, &cloned_msg_len, 1); + if(!cloned_msg) { + LM_ERR("error cloning sip message\n"); + return -1; + } - ret = 0; new_job.f = peer->callback; - new_job.msg = msg; + new_job.msg = cloned_msg; new_job.orig_peer = peer; if(!num_workers) { LM_ERR("error in add_dmq_job: no workers spawned\n"); - return -1; + goto error; } /* initialize the worker with the first one */ worker = workers; @@ -162,9 +177,16 @@ int add_dmq_job(struct sip_msg* msg, dmq_peer_t* peer) " to the least busy one [%d %d]\n", worker->pid, job_queue_size(worker->queue)); } - ret = job_queue_push(worker->queue, &new_job); + if (job_queue_push(worker->queue, &new_job)<0) { + goto error; + } lock_release(&worker->lock); - return ret; + return 0; +error: + if (cloned_msg!=NULL) { + shm_free(cloned_msg); + } + return -1; } /** diff --git a/modules/dmq/worker.h b/modules/dmq/worker.h index bda80b4..8d0e0b7 100644 --- a/modules/dmq/worker.h +++ b/modules/dmq/worker.h @@ -30,6 +30,7 @@ #include "../../atomic_ops.h" #include "../../parser/msg_parser.h" + typedef struct dmq_job { peer_callback_t f; struct sip_msg* msg;