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

Author: Daniel-Constantin Mierla miconda@gmail.com Committer: Daniel-Constantin Mierla miconda@gmail.com Date: Thu Apr 10 08:56:13 2014 +0200

tm: avoid tryin to mitigate relaying 487 for local canceling branches when a reply is forced from config

- it can result in deadlock if there is a local retransmission timeout and the response code is overwritten by admin in failure route with a t_reply() - reported by Jason Penton

---

modules/tm/h_table.h | 2 ++ modules/tm/t_cancel.c | 7 ++++++- modules/tm/timer.c | 1 + modules/tm/tm.c | 1 + 4 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/modules/tm/h_table.h b/modules/tm/h_table.h index 5790935..977f3fe 100644 --- a/modules/tm/h_table.h +++ b/modules/tm/h_table.h @@ -155,6 +155,7 @@ enum kill_reason { REQ_FWDED=1, REQ_RPLD=2, REQ_RLSD=4, REQ_EXIST=8, #define F_RB_NH_STRICT 0x200 /* next hop is a strict router */ /* must detect when neither loose nor strict flag is set -> two flags. * alternatively, 1x flag for strict/loose and 1x for loose|strict set/not */ +#define F_RB_RELAYREPLY 0x400 /* branch under relay reply condition */

/* if canceled or intended to be canceled, return true */ @@ -320,6 +321,7 @@ typedef struct async_state { #define T_ASYNC_CONTINUE (1<<12) /* Is this transaction in a continuation after being suspended */

#define T_DISABLE_INTERNAL_REPLY (1<<13) /* don't send internal negative reply */ +#define T_ADMIN_REPLY (1<<14) /* t reply sent by admin (e.g., from cfg script) */

/* unsigned short should be enough for a retr. timer: max. 65535 ms => * max retr. = 65 s which should be enough and saves us 2*2 bytes */ diff --git a/modules/tm/t_cancel.c b/modules/tm/t_cancel.c index 1b39648..c670041 100644 --- a/modules/tm/t_cancel.c +++ b/modules/tm/t_cancel.c @@ -245,7 +245,12 @@ int cancel_branch( struct cell *t, int branch, /* remove BUSY_BUFFER -- mark cancel buffer as not used */ pcbuf=&crb->buffer; /* workaround for type punning warnings */ atomic_set_long(pcbuf, 0); - if (flags & F_CANCEL_B_FAKE_REPLY){ + /* try to relay auto-generated 487 canceling response only when + * another one is not under relaying on the branch and there is + * no forced response per transaction from script */ + if((flags & F_CANCEL_B_FAKE_REPLY) + && !(irb->flags&F_RB_RELAYREPLY) + && !(t->flags&T_ADMIN_REPLY)) { LOCK_REPLIES(t); if (relay_reply(t, FAKED_REPLY, branch, 487, &tmp_cd, 1) == RPS_ERROR){ diff --git a/modules/tm/timer.c b/modules/tm/timer.c index aae9532..5e340a7 100644 --- a/modules/tm/timer.c +++ b/modules/tm/timer.c @@ -345,6 +345,7 @@ static void fake_reply(struct cell *t, int branch, int code ) do_cancel_branch = is_invite(t) && prepare_cancel_branch(t, branch, 0); /* mark branch as canceled */ t->uac[branch].request.flags|=F_RB_CANCELED; + t->uac[branch].request.flags|=F_RB_RELAYREPLY; if ( is_local(t) ) { reply_status=local_reply( t, FAKED_REPLY, branch, code, &cancel_data ); diff --git a/modules/tm/tm.c b/modules/tm/tm.c index 3bdc7fb..61d90ff 100644 --- a/modules/tm/tm.c +++ b/modules/tm/tm.c @@ -1319,6 +1319,7 @@ inline static int w_t_reply(struct sip_msg* msg, char* p1, char* p2) * the safe version would lead to a deadlock */ + t->flags |= T_ADMIN_REPLY; if (is_route_type(FAILURE_ROUTE)) { DBG("DEBUG: t_reply_unsafe called from w_t_reply\n"); ret = t_reply_unsafe(t, msg, code, r);