[sr-dev] git:kamailio_3.0: tm: t_save_lumps() verifies the route type

Module: sip-router Branch: kamailio_3.0 Commit: 47a024dacab408655a8db55aa92f9e1dfec143c9 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=47a024d… Author: Miklos Tirpak <miklos(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Thu Sep 30 10:42:57 2010 +0200 tm: t_save_lumps() verifies the route type Even though the t_save_lumps() function is registered only for request route, in some corner case, the function might be called from failure_route. (For example a failure route executes a request route block which calls this function.) This scenario resulted in overwriting the already cloned lump list which is not allowed because of the lockless read, and also resulted in a memory leak. An extra check is also added to save_msg_lumps() to catch this bug. (cherry picked from commit a7bbaf7cd83b5d044ff8c7fff7b19c7ff392da74) --- modules/tm/sip_msg.c | 8 ++++++++ modules/tm/tm.c | 22 ++++++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/modules/tm/sip_msg.c b/modules/tm/sip_msg.c index e7bebcc..ab093f8 100644 --- a/modules/tm/sip_msg.c +++ b/modules/tm/sip_msg.c @@ -116,6 +116,14 @@ int save_msg_lumps( struct sip_msg *shm_msg, struct sip_msg *pkg_msg) return -1; } +#ifdef EXTRA_DEBUG + membar_depends(); + if (shm_msg->add_rm || shm_msg->body_lumps || shm_msg->reply_lump) { + LOG(L_ERR, "ERROR: save_msg_lumps: BUG, trying to overwrite the already cloned lumps\n"); + return -1; + } +#endif + /* needless to clone the lumps for ACK, they will not be used again */ if (shm_msg->REQ_METHOD == METHOD_ACK) return 0; diff --git a/modules/tm/tm.c b/modules/tm/tm.c index 5ed5a65..9e9dfbe 100644 --- a/modules/tm/tm.c +++ b/modules/tm/tm.c @@ -1811,17 +1811,19 @@ static int w_t_save_lumps(struct sip_msg* msg, char* foo, char* bar) #ifdef POSTPONE_MSG_CLONING struct cell *t; - t=get_t(); - if (!t || t==T_UNDEFINED) { - LOG(L_ERR, "ERROR: w_t_save_lumps: transaction has not been created yet\n"); - return -1; - } + if (is_route_type(REQUEST_ROUTE)) { + t=get_t(); + if (!t || t==T_UNDEFINED) { + LOG(L_ERR, "ERROR: w_t_save_lumps: transaction has not been created yet\n"); + return -1; + } - if (save_msg_lumps(t->uas.request, msg)) { - LOG(L_ERR, "ERROR: w_t_save_lumps: " - "failed to save the message lumps\n"); - return -1; - } + if (save_msg_lumps(t->uas.request, msg)) { + LOG(L_ERR, "ERROR: w_t_save_lumps: " + "failed to save the message lumps\n"); + return -1; + } + } /* else nothing to do, the lumps have already been saved */ return 1; #else LOG(L_ERR, "ERROR: w_t_save_lumps: POSTPONE_MSG_CLONING is not defined,"