[sr-dev] git:master: tm: t_save_lumps() verifies the route type

[Miklos Tirpak]

Module: sip-router
Branch: master
Commit: a7bbaf7cd83b5d044ff8c7fff7b19c7ff392da74
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=a7bbaf7cd83b5d044ff8c7fff7b19c7ff392da74

Author: Miklos Tirpak <miklos at iptel.org>
Committer: Miklos Tirpak <miklos at 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.

---

 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 c5cd841..e5ca753 100644
--- a/modules/tm/tm.c
+++ b/modules/tm/tm.c
@@ -1885,17 +1885,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,"