sipwise
/
kamailio
mirror of https://github.com/sipwise/kamailio.git
1
0
Fork 0
Code Issues Releases Wiki Activity

TT#10122 tm: fix race condition

Browse Source 
Change-Id: Iedf68425ea66e676b434cb368a909c4d15f38b26
changes/39/11139/3
Victor Seva 8 years ago committed by Richard Fuchs
parent 37a79ead36
commit 86e3621307
2 changed files with 398 additions and 0 deletions
Show Stats Download Patch File Download Diff File

1
debian/patches/series vendored
Unescape View File

@ -41,3 +41,4 @@ sipwise/sca-fix-sca_call_info_update-params.patch
sipwise/sca-use-onhold_bflag-value-only-when-set.patch
##
sipwise/rtpengine-sockets-list-fix.patch
sipwise/tm-deep-cloning-of-the-request-for-fake-environment.patch

397
debian/patches/sipwise/tm-deep-cloning-of-the-request-for-fake-environment.patch vendored
Unescape View File

@ -0,0 +1,397 @@
From 2e2f75c04e22c9f9dfdf55338212dfeb2e0bbf47 Mon Sep 17 00:00:00 2001
From: Victor Seva <linuxmaniac@torreviejawireless.org>
Date: Mon, 6 Feb 2017 19:41:15 +0100
Subject: [PATCH] tm: deep cloning of the request for fake environment
---
modules/tm/t_append_branches.c | 24 ++++++-----
modules/tm/t_reply.c | 96 ++++++++++++++++++++++++------------------
modules/tm/t_reply.h | 6 +--
modules/tm/t_suspend.c | 24 ++++++-----
4 files changed, 86 insertions(+), 64 deletions(-)
diff --git a/modules/tm/t_append_branches.c b/modules/tm/t_append_branches.c
index f745e755d..96cd40f78 100644
--- a/modules/tm/t_append_branches.c
+++ b/modules/tm/t_append_branches.c
@@ -49,8 +49,9 @@
int t_append_branches(void) {
struct cell *t = NULL;
struct sip_msg *orig_msg = NULL;
- static struct sip_msg faked_req;
-
+ struct sip_msg *faked_req;
+ int faked_req_len = 0;
+
short outgoings;
int success_branch;
@@ -109,13 +110,14 @@ int t_append_branches(void) {
set_branch_route(t->on_branch_delayed);
}
- if (!fake_req(&faked_req, orig_msg, 0, NULL)) {
+ faked_req = fake_req(orig_msg, 0, NULL, &faked_req_len);
+ if (faked_req==NULL) {
LOG(L_ERR, "ERROR: t_append_branches: fake_req failed\n");
return -1;
}
/* fake also the env. conforming to the fake msg */
- faked_env( t, &faked_req, 0);
+ faked_env( t, faked_req, 0);
/* DONE with faking ;-) -> run the failure handlers */
init_branch_iterator();
@@ -139,9 +141,9 @@ int t_append_branches(void) {
continue;
setbflagsval(0, bflags);
- new_branch=add_uac( t, &faked_req, &current_uri,
+ new_branch=add_uac( t, faked_req, &current_uri,
(dst_uri.len) ? (&dst_uri) : &current_uri,
- &path, 0, si, faked_req.fwd_send_flags,
+ &path, 0, si, faked_req->fwd_send_flags,
PROTO_NONE, (dst_uri.len)?0:UAC_SKIP_BR_DST_F, &instance,
&ruid, &location_ua);
@@ -163,7 +165,7 @@ int t_append_branches(void) {
setbflagsval(0, backup_bflags);
/* update message flags, if changed in branch route */
- t->uas.request->flags = faked_req.flags;
+ t->uas.request->flags = faked_req->flags;
if (added_branches==0) {
if(lowest_ret!=E_CFG)
@@ -180,14 +182,14 @@ int t_append_branches(void) {
for (i=outgoings; i<t->nr_of_outgoings; i++) {
if (added_branches & (1<<i)) {
- branch_ret=t_send_branch(t, i, &faked_req , 0, 0 /* replies are already locked */ );
+ branch_ret=t_send_branch(t, i, faked_req , 0, 0 /* replies are already locked */ );
if (branch_ret>=0){ /* some kind of success */
if (branch_ret==i) { /* success */
success_branch++;
if (unlikely(has_tran_tmcbs(t, TMCB_REQUEST_OUT)))
run_trans_callbacks_with_buf( TMCB_REQUEST_OUT,
&t->uac[nr_branches].request,
- &faked_req, 0, -orig_msg->REQ_METHOD);
+ faked_req, 0, -orig_msg->REQ_METHOD);
}
else /* new branch added */
added_branches |= 1<<branch_ret;
@@ -217,7 +219,7 @@ canceled:
/* restore backup flags from initial env */
setbflagsval(0, backup_bflags);
/* update message flags, if changed in branch route */
- t->uas.request->flags = faked_req.flags;
+ t->uas.request->flags = faked_req->flags;
/* if needed unlock transaction's replies */
/* restore the number of outgoing branches
* since new branches have not been completed */
@@ -227,7 +229,7 @@ canceled:
done:
/* restore original environment and free the fake msg */
faked_env( t, 0, 0);
- free_faked_req(&faked_req,t);
+ free_faked_req(faked_req, faked_req_len);
if (likely(replies_locked)) {
replies_locked = 0;
diff --git a/modules/tm/t_reply.c b/modules/tm/t_reply.c
index cf56de701..c407ec9a7 100644
--- a/modules/tm/t_reply.c
+++ b/modules/tm/t_reply.c
@@ -70,6 +70,7 @@
#include "t_lookup.h"
#include "t_fwd.h"
#include "../../fix_lumps.h"
+#include "../../sip_msg_clone.h"
#include "../../sr_compat.h"
#include "../../receive.h"
#include "../../onsend.h"
@@ -843,17 +844,20 @@ int fake_req_clone_str_helper(str *src, str *dst, char *txt)
}
/**
- * fake a semi-private sip message using transaction's shared memory message
+ * fake a private sip message using transaction's shared memory message
*/
-int fake_req(struct sip_msg *faked_req,
- struct sip_msg *shmem_msg, int extra_flags, struct ua_client *uac)
+struct sip_msg * fake_req(struct sip_msg *shmem_msg, int extra_flags,
+ struct ua_client *uac, int *len)
{
- /* on_failure_reply faked msg now copied from shmem msg (as opposed
- * to zero-ing) -- more "read-only" actions (exec in particular) will
- * work from reply_route as they will see msg->from, etc.; caution,
- * rw actions may append some pkg stuff to msg, which will possibly be
- * never released (shmem is released in a single block) */
- memcpy( faked_req, shmem_msg, sizeof(struct sip_msg));
+ struct sip_msg *faked_req;
+ /* make a clone so eventual new parsed headers in pkg are not visible
+ * to other processes -- other attributes should be already parsed,
+ * available in the req structure and propagated by cloning */
+ faked_req = sip_msg_shm_clone(shmem_msg, len, 1);
+ if(faked_req==NULL) {
+ LM_ERR("failed to clone the request\n");
+ return NULL;
+ }
/* if we set msg_id to something different from current's message
* id, the first t_fork will properly clean new branch URIs */
@@ -883,7 +887,7 @@ int fake_req(struct sip_msg *faked_req,
if(uac) setbflagsval(0, uac->branch_flags);
else setbflagsval(0, 0);
- return 1;
+ return faked_req;
error02:
if (faked_req->dst_uri.s) {
@@ -898,12 +902,15 @@ error01:
faked_req->path_vec.len = 0;
}
error00:
- return 0;
+ shm_free(faked_req);
+ return NULL;
}
-void free_faked_req(struct sip_msg *faked_req, struct cell *t)
+void free_faked_req(struct sip_msg *faked_req, int len)
{
struct hdr_field *hdr;
+ void *mstart = faked_req;
+ void *mend =((char *) faked_req) + len;
reset_new_uri(faked_req);
reset_dst_uri(faked_req);
@@ -916,9 +923,8 @@ void free_faked_req(struct sip_msg *faked_req, struct cell *t)
/* free header's parsed structures that were added by failure handlers */
for( hdr=faked_req->headers ; hdr ; hdr=hdr->next ) {
if ( hdr->parsed && hdr_allocs_parse(hdr) &&
- (hdr->parsed<(void*)t->uas.request ||
- hdr->parsed>=(void*)t->uas.end_request)) {
- /* header parsed filed doesn't point inside uas.request memory
+ (hdr->parsed<mstart || hdr->parsed>=mend)) {
+ /* header parsed filed doesn't point inside fake memory
* chunck -> it was added by failure funcs.-> free it as pkg */
DBG("DBG:free_faked_req: removing hdr->parsed %d\n",
hdr->type);
@@ -939,14 +945,17 @@ void free_faked_req(struct sip_msg *faked_req, struct cell *t)
reset_ruid(faked_req);
reset_ua(faked_req);
msg_ldata_reset(faked_req);
-}
+ /* free shared block */
+ shm_free(faked_req);
+}
/* return 1 if a failure_route processes */
int run_failure_handlers(struct cell *t, struct sip_msg *rpl,
int code, int extra_flags)
{
- static struct sip_msg faked_req;
+ struct sip_msg *faked_req;
+ int faked_req_len = 0;
struct sip_msg *shmem_msg = t->uas.request;
int on_failure;
@@ -967,16 +976,18 @@ int run_failure_handlers(struct cell *t, struct sip_msg *rpl,
return 1;
}
- if (!fake_req(&faked_req, shmem_msg, extra_flags, &t->uac[picked_branch])) {
+ faked_req = fake_req(shmem_msg, extra_flags, &t->uac[picked_branch],
+ &faked_req_len);
+ if (faked_req==NULL) {
LOG(L_ERR, "ERROR: run_failure_handlers: fake_req failed\n");
return 0;
}
/* fake also the env. conforming to the fake msg */
- faked_env( t, &faked_req, 0);
+ faked_env( t, faked_req, 0);
/* DONE with faking ;-) -> run the failure handlers */
if (unlikely(has_tran_tmcbs( t, TMCB_ON_FAILURE)) ) {
- run_trans_callbacks( TMCB_ON_FAILURE, t, &faked_req, rpl, code);
+ run_trans_callbacks( TMCB_ON_FAILURE, t, faked_req, rpl, code);
}
if (on_failure) {
/* avoid recursion -- if failure_route forwards, and does not
@@ -985,22 +996,23 @@ int run_failure_handlers(struct cell *t, struct sip_msg *rpl,
t->on_failure=0;
/* if continuing on timeout of a suspended transaction, reset the flag */
t->flags &= ~T_ASYNC_SUSPENDED;
- if (exec_pre_script_cb(&faked_req, FAILURE_CB_TYPE)>0) {
+ if (exec_pre_script_cb(faked_req, FAILURE_CB_TYPE)>0) {
/* run a failure_route action if some was marked */
- if (run_top_route(failure_rt.rlist[on_failure], &faked_req, 0)<0)
+ if (run_top_route(failure_rt.rlist[on_failure], faked_req, 0)<0)
LOG(L_ERR, "ERROR: run_failure_handlers: Error in run_top_route\n");
- exec_post_script_cb(&faked_req, FAILURE_CB_TYPE);
+ exec_post_script_cb(faked_req, FAILURE_CB_TYPE);
}
/* update message flags, if changed in failure route */
- t->uas.request->flags = faked_req.flags;
+ t->uas.request->flags = faked_req->flags;
}
- /* restore original environment and free the fake msg */
+ /* restore original environment */
faked_env( t, 0, 0);
- free_faked_req(&faked_req,t);
-
/* if failure handler changed flag, update transaction context */
- shmem_msg->flags = faked_req.flags;
+ shmem_msg->flags = faked_req->flags;
+ /* free the fake msg */
+ free_faked_req(faked_req, faked_req_len);
+
return 1;
}
@@ -1009,7 +1021,8 @@ int run_failure_handlers(struct cell *t, struct sip_msg *rpl,
int run_branch_failure_handlers(struct cell *t, struct sip_msg *rpl,
int code, int extra_flags)
{
- static struct sip_msg faked_req;
+ struct sip_msg *faked_req;
+ int faked_req_len = 0;
struct sip_msg *shmem_msg = t->uas.request;
int on_branch_failure;
@@ -1030,37 +1043,40 @@ int run_branch_failure_handlers(struct cell *t, struct sip_msg *rpl,
return 1;
}
- if (!fake_req(&faked_req, shmem_msg, extra_flags, &t->uac[picked_branch])) {
+ faked_req = fake_req(shmem_msg, extra_flags, &t->uac[picked_branch],
+ &faked_req_len);
+ if (faked_req==NULL) {
LOG(L_ERR, "fake_req failed\n");
return 0;
}
/* fake also the env. conforming to the fake msg */
- faked_env( t, &faked_req, 0);
+ faked_env( t, faked_req, 0);
set_route_type(BRANCH_FAILURE_ROUTE);
set_t(t, picked_branch);
/* DONE with faking ;-) -> run the branch_failure handlers */
if (unlikely(has_tran_tmcbs( t, TMCB_ON_BRANCH_FAILURE)) ) {
- run_trans_callbacks( TMCB_ON_BRANCH_FAILURE, t, &faked_req, rpl, code);
+ run_trans_callbacks( TMCB_ON_BRANCH_FAILURE, t, faked_req, rpl, code);
}
if (on_branch_failure >= 0) {
t->on_branch_failure = 0;
- if (exec_pre_script_cb(&faked_req, BRANCH_FAILURE_CB_TYPE)>0) {
+ if (exec_pre_script_cb(faked_req, BRANCH_FAILURE_CB_TYPE)>0) {
/* run a branch_failure_route action if some was marked */
- if (run_top_route(event_rt.rlist[on_branch_failure], &faked_req, 0)<0)
+ if (run_top_route(event_rt.rlist[on_branch_failure], faked_req, 0)<0)
LOG(L_ERR, "error in run_top_route\n");
- exec_post_script_cb(&faked_req, BRANCH_FAILURE_CB_TYPE);
+ exec_post_script_cb(faked_req, BRANCH_FAILURE_CB_TYPE);
}
/* update message flags, if changed in branch_failure route */
- t->uas.request->flags = faked_req.flags;
+ t->uas.request->flags = faked_req->flags;
}
- /* restore original environment and free the fake msg */
+ /* restore original environment */
faked_env( t, 0, 0);
- free_faked_req(&faked_req,t);
-
/* if branch_failure handler changed flag, update transaction context */
- shmem_msg->flags = faked_req.flags;
+ shmem_msg->flags = faked_req->flags;
+ /* free the fake msg */
+ free_faked_req(faked_req, faked_req_len);
+
return 1;
}
diff --git a/modules/tm/t_reply.h b/modules/tm/t_reply.h
index 91b2ac839..af99187e7 100644
--- a/modules/tm/t_reply.h
+++ b/modules/tm/t_reply.h
@@ -227,10 +227,10 @@ void t_drop_replies(int v);
void rpc_reply(rpc_t* rpc, void* c);
void faked_env( struct cell *t,struct sip_msg *msg, int is_async_env);
-int fake_req(struct sip_msg *faked_req,
- struct sip_msg *shmem_msg, int extra_flags, struct ua_client *uac);
+struct sip_msg * fake_req(struct sip_msg *shmem_msg,
+ int extra_flags, struct ua_client *uac, int *len);
-void free_faked_req(struct sip_msg *faked_req, struct cell *t);
+void free_faked_req(struct sip_msg *faked_req, int len);
typedef int (*tget_picked_f)(void);
int t_get_picked_branch(void);
diff --git a/modules/tm/t_suspend.c b/modules/tm/t_suspend.c
index 6bb25473b..27f54569a 100644
--- a/modules/tm/t_suspend.c
+++ b/modules/tm/t_suspend.c
@@ -166,7 +166,8 @@ int t_continue(unsigned int hash_index, unsigned int label,
struct action *route)
{
struct cell *t;
- struct sip_msg faked_req;
+ struct sip_msg *faked_req;
+ int faked_req_len = 0;
struct cancel_info cancel_data;
int branch;
struct ua_client *uac =NULL;
@@ -267,31 +268,34 @@ int t_continue(unsigned int hash_index, unsigned int label,
*/
/* fake the request and the environment, like in failure_route */
- if (!fake_req(&faked_req, t->uas.request, 0 /* extra flags */, uac)) {
+ faked_req = fake_req(t->uas.request, 0 /* extra flags */, uac,
+ &faked_req_len);
+ if (faked_req==NULL) {
LM_ERR("building fake_req failed\n");
ret = -1;
goto kill_trans;
}
- faked_env( t, &faked_req, 1);
+ faked_env( t, faked_req, 1);
route_type_bk = get_route_type();
set_route_type(FAILURE_ROUTE);
/* execute the pre/post -script callbacks based on original route block */
- if (exec_pre_script_cb(&faked_req, cb_type)>0) {
- if (run_top_route(route, &faked_req, 0)<0)
+ if (exec_pre_script_cb(faked_req, cb_type)>0) {
+ if (run_top_route(route, faked_req, 0)<0)
LM_ERR("failure inside run_top_route\n");
- exec_post_script_cb(&faked_req, cb_type);
+ exec_post_script_cb(faked_req, cb_type);
}
set_route_type(route_type_bk);
/* TODO: save_msg_lumps should clone the lumps to shm mem */
- /* restore original environment and free the fake msg */
+ /* restore original environment */
faked_env( t, 0, 1);
- free_faked_req(&faked_req, t);
-
/* update the flags */
- t->uas.request->flags = faked_req.flags;
+ t->uas.request->flags = faked_req->flags;
+ /* free the fake msg */
+ free_faked_req(faked_req, faked_req_len);
+
if (t->uas.status < 200) {
/* No final reply has been sent yet.
--
2.11.0
Write Preview
Loading…
Cancel
Save