[svn-commits] rmudgett: branch 13 r430817 - in /branches/13: ./ channels/ configs/samples/ ...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Tue Jan 20 10:46:29 CST 2015
Author: rmudgett
Date: Tue Jan 20 10:46:16 2015
New Revision: 430817
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=430817
Log:
CHANNEL(peer), chan_iax2, res_fax, SNMP agent: Fix deadlock from reaching across a bridge.
Calling ast_channel_bridge_peer() cannot be done while holding any channel
locks. The reported issue hit the deadlock in chan_iax2, but an audit of
the ast_channel_bridge_peer() calls found three more locations where the
same deadlock can occur.
* Made CHANNEL(peer), res_fax, and the SNMP agent not call
ast_channel_bridge_peer() with any channel locked. For CHANNEL(peer) I
had to rework the logic to not hold the channel lock.
* Made chan_iax2 no longer call ast_channel_bridge_peer(). It was done
for legacy reasons that no longer apply.
* Removed the iax.conf forcejitterbuffer option. It is now always enabled
when the jitterbuffer option is enabled. If you put a jitter buffer on a
channel it will be on the channel.
ASTERISK-24600 #close
Reported by: Jeff Collell
Review: https://reviewboard.asterisk.org/r/4342/
Modified:
branches/13/UPGRADE.txt
branches/13/channels/chan_iax2.c
branches/13/configs/samples/iax.conf.sample
branches/13/funcs/func_channel.c
branches/13/res/res_fax.c
branches/13/res/snmp/agent.c
Modified: branches/13/UPGRADE.txt
URL: http://svnview.digium.com/svn/asterisk/branches/13/UPGRADE.txt?view=diff&rev=430817&r1=430816&r2=430817
==============================================================================
--- branches/13/UPGRADE.txt (original)
+++ branches/13/UPGRADE.txt Tue Jan 20 10:46:16 2015
@@ -37,6 +37,11 @@
- The CALLERID(ani2) value for incoming calls is now populated in featdmf
signaling mode. The information was previously discarded.
+chan_iax2:
+ - The iax.conf forcejitterbuffer option has been removed. It is now always
+ forced if you set iax.conf jitterbuffer=yes. If you put a jitter buffer
+ on a channel it will be on the channel.
+
From 13.0.0 to 13.1.0:
ARI:
Modified: branches/13/channels/chan_iax2.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/chan_iax2.c?view=diff&rev=430817&r1=430816&r2=430817
==============================================================================
--- branches/13/channels/chan_iax2.c (original)
+++ branches/13/channels/chan_iax2.c Tue Jan 20 10:46:16 2015
@@ -455,7 +455,6 @@
#define IAX_RTCACHEFRIENDS (uint64_t)(1 << 17) /*!< let realtime stay till your reload */
#define IAX_RTUPDATE (uint64_t)(1 << 18) /*!< Send a realtime update */
#define IAX_RTAUTOCLEAR (uint64_t)(1 << 19) /*!< erase me on expire */
-#define IAX_FORCEJITTERBUF (uint64_t)(1 << 20) /*!< Force jitterbuffer, even when bridged to a channel that can take jitter */
#define IAX_RTIGNOREREGEXPIRE (uint64_t)(1 << 21) /*!< When using realtime, ignore registration expiration */
#define IAX_TRUNKTIMESTAMPS (uint64_t)(1 << 22) /*!< Send trunk timestamps */
#define IAX_TRANSFERMEDIA (uint64_t)(1 << 23) /*!< When doing IAX2 transfers, transfer media only */
@@ -3180,7 +3179,7 @@
iaxs[x]->pingid = iax2_sched_add(sched, ping_time * 1000, send_ping, (void *)(long)x);
iaxs[x]->lagid = iax2_sched_add(sched, lagrq_time * 1000, send_lagrq, (void *)(long)x);
iaxs[x]->amaflags = amaflags;
- ast_copy_flags64(iaxs[x], &globalflags, IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
+ ast_copy_flags64(iaxs[x], &globalflags, IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
ast_string_field_set(iaxs[x], accountcode, accountcode);
ast_string_field_set(iaxs[x], mohinterpret, mohinterpret);
ast_string_field_set(iaxs[x], mohsuggest, mohsuggest);
@@ -4191,8 +4190,6 @@
int type, len;
int ret;
int needfree = 0;
- struct ast_channel *owner = NULL;
- RAII_VAR(struct ast_channel *, bridge, NULL, ast_channel_cleanup);
/*
* Clear fr->af.data if there is no data in the buffer. Things
@@ -4231,45 +4228,6 @@
*tsout = fr->ts;
__do_deliver(fr);
return -1;
- }
-
- iax2_lock_owner(fr->callno);
- if (!iaxs[fr->callno]) {
- /* The call dissappeared so discard this frame that we could not send. */
- iax2_frame_free(fr);
- return -1;
- }
- if ((owner = iaxs[fr->callno]->owner)) {
- bridge = ast_channel_bridge_peer(owner);
- }
-
- /* if the user hasn't requested we force the use of the jitterbuffer, and we're bridged to
- * a channel that can accept jitter, then flush and suspend the jb, and send this frame straight through */
- if ( (!ast_test_flag64(iaxs[fr->callno], IAX_FORCEJITTERBUF)) && owner && bridge && (ast_channel_tech(bridge)->properties & AST_CHAN_TP_WANTSJITTER) ) {
- jb_frame frame;
-
- ast_channel_unlock(owner);
-
- /* deliver any frames in the jb */
- while (jb_getall(iaxs[fr->callno]->jb, &frame) == JB_OK) {
- __do_deliver(frame.data);
- /* __do_deliver() can make the call disappear */
- if (!iaxs[fr->callno])
- return -1;
- }
-
- jb_reset(iaxs[fr->callno]->jb);
-
- AST_SCHED_DEL(sched, iaxs[fr->callno]->jbid);
-
- /* deliver this frame now */
- if (tsout)
- *tsout = fr->ts;
- __do_deliver(fr);
- return -1;
- }
- if (owner) {
- ast_channel_unlock(owner);
}
/* insert into jitterbuffer */
@@ -4670,7 +4628,7 @@
if (peer->maxms && ((peer->lastms > peer->maxms) || (peer->lastms < 0)))
goto return_unref;
- ast_copy_flags64(cai, peer, IAX_SENDANI | IAX_TRUNK | IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
+ ast_copy_flags64(cai, peer, IAX_SENDANI | IAX_TRUNK | IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
cai->maxtime = peer->maxms;
cai->capability = peer->capability;
cai->encmethods = peer->encmethods;
@@ -7977,7 +7935,7 @@
iaxs[callno]->amaflags = user->amaflags;
if (!ast_strlen_zero(user->language))
ast_string_field_set(iaxs[callno], language, user->language);
- ast_copy_flags64(iaxs[callno], user, IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
+ ast_copy_flags64(iaxs[callno], user, IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
/* Keep this check last */
if (!ast_strlen_zero(user->dbsecret)) {
char *family, *key=NULL;
@@ -12460,7 +12418,7 @@
memset(&cai, 0, sizeof(cai));
cai.capability = iax2_capability;
- ast_copy_flags64(&cai, &globalflags, IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
+ ast_copy_flags64(&cai, &globalflags, IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
/* Populate our address from the given */
if (create_addr(pds.peer, NULL, &addr, &cai)) {
@@ -12482,7 +12440,7 @@
}
/* If this is a trunk, update it now */
- ast_copy_flags64(iaxs[callno], &cai, IAX_TRUNK | IAX_SENDANI | IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
+ ast_copy_flags64(iaxs[callno], &cai, IAX_TRUNK | IAX_SENDANI | IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
if (ast_test_flag64(&cai, IAX_TRUNK)) {
int new_callno;
if ((new_callno = make_trunk(callno, 1)) != -1)
@@ -12800,7 +12758,7 @@
if (peer) {
if (firstpass) {
- ast_copy_flags64(peer, &globalflags, IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
+ ast_copy_flags64(peer, &globalflags, IAX_USEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
peer->encmethods = iax2_encryption;
peer->adsi = adsi;
ast_string_field_set(peer, secret, "");
@@ -12887,8 +12845,6 @@
ast_set_flags_to64(peer, IAX_NOTRANSFER|IAX_TRANSFERMEDIA, IAX_NOTRANSFER);
} else if (!strcasecmp(v->name, "jitterbuffer")) {
ast_set2_flag64(peer, ast_true(v->value), IAX_USEJITTERBUF);
- } else if (!strcasecmp(v->name, "forcejitterbuffer")) {
- ast_set2_flag64(peer, ast_true(v->value), IAX_FORCEJITTERBUF);
} else if (!strcasecmp(v->name, "host")) {
if (!strcasecmp(v->value, "dynamic")) {
/* They'll register with us */
@@ -13134,7 +13090,7 @@
user->calltoken_required = CALLTOKEN_DEFAULT;
ast_string_field_set(user, name, name);
ast_string_field_set(user, language, language);
- ast_copy_flags64(user, &globalflags, IAX_USEJITTERBUF | IAX_FORCEJITTERBUF | IAX_CODEC_USER_FIRST | IAX_CODEC_NOPREFS | IAX_CODEC_NOCAP | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
+ ast_copy_flags64(user, &globalflags, IAX_USEJITTERBUF | IAX_CODEC_USER_FIRST | IAX_CODEC_NOPREFS | IAX_CODEC_NOCAP | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE | IAX_FORCE_ENCRYPT);
ast_clear_flag64(user, IAX_HASCALLERID);
ast_string_field_set(user, cid_name, "");
ast_string_field_set(user, cid_num, "");
@@ -13216,8 +13172,6 @@
ast_set2_flag64(user, ast_true(v->value), IAX_IMMEDIATE);
} else if (!strcasecmp(v->name, "jitterbuffer")) {
ast_set2_flag64(user, ast_true(v->value), IAX_USEJITTERBUF);
- } else if (!strcasecmp(v->name, "forcejitterbuffer")) {
- ast_set2_flag64(user, ast_true(v->value), IAX_FORCEJITTERBUF);
} else if (!strcasecmp(v->name, "dbsecret")) {
ast_string_field_set(user, dbsecret, v->value);
} else if (!strcasecmp(v->name, "secret")) {
@@ -13430,7 +13384,7 @@
amaflags = 0;
delayreject = 0;
ast_clear_flag64((&globalflags), IAX_NOTRANSFER | IAX_TRANSFERMEDIA | IAX_USEJITTERBUF |
- IAX_FORCEJITTERBUF | IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
+ IAX_SENDCONNECTEDLINE | IAX_RECVCONNECTEDLINE);
delete_users();
ao2_callback(callno_limits, OBJ_NODATA, addr_range_delme_cb, NULL);
ao2_callback(calltoken_ignores, OBJ_NODATA, addr_range_delme_cb, NULL);
@@ -13661,8 +13615,6 @@
}
} else if (!strcasecmp(v->name, "jitterbuffer"))
ast_set2_flag64((&globalflags), ast_true(v->value), IAX_USEJITTERBUF);
- else if (!strcasecmp(v->name, "forcejitterbuffer"))
- ast_set2_flag64((&globalflags), ast_true(v->value), IAX_FORCEJITTERBUF);
else if (!strcasecmp(v->name, "delayreject"))
delayreject = ast_true(v->value);
else if (!strcasecmp(v->name, "allowfwdownload"))
Modified: branches/13/configs/samples/iax.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/13/configs/samples/iax.conf.sample?view=diff&rev=430817&r1=430816&r2=430817
==============================================================================
--- branches/13/configs/samples/iax.conf.sample (original)
+++ branches/13/configs/samples/iax.conf.sample Tue Jan 20 10:46:16 2015
@@ -170,12 +170,6 @@
; jitterbuffer=yes|no: global default as to whether you want
; the jitter buffer at all.
;
-; forcejitterbuffer=yes|no: in the ideal world, when we bridge VoIP channels
-; we don't want to do jitterbuffering on the switch, since the endpoints
-; can each handle this. However, some endpoints may have poor jitterbuffers
-; themselves, so this option will force * to always jitterbuffer, even in this
-; case.
-;
; maxjitterbuffer: a maximum size for the jitter buffer.
; Setting a reasonable maximum here will prevent the call delay
; from rising to silly values in extreme situations; you'll hear
@@ -202,7 +196,6 @@
;
jitterbuffer=no
-forcejitterbuffer=no
;maxjitterbuffer=1000
;maxjitterinterps=10
;resyncthreshold=1000
Modified: branches/13/funcs/func_channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/funcs/func_channel.c?view=diff&rev=430817&r1=430816&r2=430817
==============================================================================
--- branches/13/funcs/func_channel.c (original)
+++ branches/13/funcs/func_channel.c Tue Jan 20 10:46:16 2015
@@ -508,22 +508,34 @@
}
ast_channel_unlock(chan);
} else if (!strcasecmp(data, "peer")) {
- RAII_VAR(struct ast_channel *, p, NULL, ast_channel_cleanup);
-
- ast_channel_lock(chan);
- p = ast_channel_bridge_peer(chan);
- if (p || ast_channel_tech(chan)) /* dummy channel? if so, we hid the peer name in the language */
- ast_copy_string(buf, (p ? ast_channel_name(p) : ""), len);
- else {
- /* a dummy channel can still pass along bridged peer info via
- the BRIDGEPEER variable */
- const char *pname = pbx_builtin_getvar_helper(chan, "BRIDGEPEER");
- if (!ast_strlen_zero(pname))
- ast_copy_string(buf, pname, len); /* a horrible kludge, but... how else? */
- else
- buf[0] = 0;
- }
- ast_channel_unlock(chan);
+ struct ast_channel *peer;
+
+ peer = ast_channel_bridge_peer(chan);
+ if (peer) {
+ /* Only real channels could have a bridge peer this way. */
+ ast_channel_lock(peer);
+ ast_copy_string(buf, ast_channel_name(peer), len);
+ ast_channel_unlock(peer);
+ ast_channel_unref(peer);
+ } else {
+ buf[0] = '\0';
+ ast_channel_lock(chan);
+ if (!ast_channel_tech(chan)) {
+ const char *pname;
+
+ /*
+ * A dummy channel can still pass along bridged peer info
+ * via the BRIDGEPEER variable.
+ *
+ * A horrible kludge, but... how else?
+ */
+ pname = pbx_builtin_getvar_helper(chan, "BRIDGEPEER");
+ if (!ast_strlen_zero(pname)) {
+ ast_copy_string(buf, pname, len);
+ }
+ }
+ ast_channel_unlock(chan);
+ }
} else if (!strcasecmp(data, "uniqueid")) {
locked_copy_string(chan, buf, ast_channel_uniqueid(chan), len);
} else if (!strcasecmp(data, "transfercapability")) {
Modified: branches/13/res/res_fax.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_fax.c?view=diff&rev=430817&r1=430816&r2=430817
==============================================================================
--- branches/13/res/res_fax.c (original)
+++ branches/13/res/res_fax.c Tue Jan 20 10:46:16 2015
@@ -2888,6 +2888,7 @@
static int fax_gateway_start(struct fax_gateway *gateway, struct ast_fax_session_details *details, struct ast_channel *chan)
{
struct ast_fax_session *s;
+ int start_res;
/* create the FAX session */
if (!(s = fax_session_new(details, chan, gateway->s, gateway->token))) {
@@ -2908,7 +2909,10 @@
gateway->s = s;
gateway->token = NULL;
- if (gateway->s->tech->start_session(gateway->s) < 0) {
+ ast_channel_unlock(chan);
+ start_res = gateway->s->tech->start_session(gateway->s);
+ ast_channel_lock(chan);
+ if (start_res < 0) {
ast_string_field_set(details, result, "FAILED");
ast_string_field_set(details, resultstr, "error starting gateway session");
ast_string_field_set(details, error, "INIT_ERROR");
Modified: branches/13/res/snmp/agent.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/snmp/agent.c?view=diff&rev=430817&r1=430816&r2=430817
==============================================================================
--- branches/13/res/snmp/agent.c (original)
+++ branches/13/res/snmp/agent.c Tue Jan 20 10:46:16 2015
@@ -298,12 +298,18 @@
}
break;
case ASTCHANBRIDGE:
- if ((bridge = ast_channel_bridge_peer(chan)) != NULL) {
+ ast_channel_unlock(chan);
+ bridge = ast_channel_bridge_peer(chan);
+ if (bridge) {
+ ast_channel_lock(bridge);
ast_copy_string(string_ret, ast_channel_name(bridge), sizeof(string_ret));
- *var_len = strlen(string_ret);
- ret = (u_char *)string_ret;
+ ast_channel_unlock(bridge);
ast_channel_unref(bridge);
- }
+
+ *var_len = strlen(string_ret);
+ ret = (u_char *)string_ret;
+ }
+ ast_channel_lock(chan);
break;
case ASTCHANMASQ:
if (ast_channel_masq(chan) && !ast_strlen_zero(ast_channel_name(ast_channel_masq(chan)))) {
More information about the svn-commits
mailing list