[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.
Charles Chance
charles.chance at sipcentric.com
Wed Oct 23 13:03:14 CEST 2013
Module: sip-router
Branch: master
Commit: 3b028d308fac3a4f7ae1e74021882657753f2ee8
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=3b028d308fac3a4f7ae1e74021882657753f2ee8
Author: Charles Chance <charles.chance at sipcentric.com>
Committer: Charles Chance <charles.chance at sipcentric.com>
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;
More information about the sr-dev
mailing list