diff options
author | Richard Mudgett <rmudgett@digium.com> | 2012-06-11 17:34:08 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2012-06-11 17:34:08 +0000 |
commit | 72eb8eb1e7ee0e097c1124a9c12cc984a670ee24 (patch) | |
tree | c5ccd4f6f09127b22eeaf49b7f37585b1e48d723 | |
parent | c6142cf2cc7312122abcf74d6ab1daf0ad1d72bf (diff) |
Fix deadlock potential with ast_set_hangupsource() calls.
Calling ast_set_hangupsource() with the channel lock held can result in a
deadlock because the function also locks the bridged channel.
(issue ASTERISK-19537)
(closes issue AST-891)
Reported by: Guenther Kelleter
Tested by: Guenther Kelleter
(closes issue ASTERISK-19801)
Reported by: Alec Davis
........
Merged revisions 368759 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 368760 from http://svn.asterisk.org/svn/asterisk/branches/10
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@368772 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r-- | channels/chan_dahdi.c | 13 | ||||
-rw-r--r-- | channels/chan_iax2.c | 15 | ||||
-rw-r--r-- | channels/chan_sip.c | 51 | ||||
-rw-r--r-- | channels/sig_analog.c | 11 | ||||
-rw-r--r-- | include/asterisk/channel.h | 2 | ||||
-rw-r--r-- | main/channel.c | 10 |
6 files changed, 83 insertions, 19 deletions
diff --git a/channels/chan_dahdi.c b/channels/chan_dahdi.c index e6c39d28e..6c5b00ef0 100644 --- a/channels/chan_dahdi.c +++ b/channels/chan_dahdi.c @@ -8955,13 +8955,18 @@ static struct ast_frame *__dahdi_exception(struct ast_channel *ast) f = &p->subs[idx].f; return f; } + f = dahdi_handle_event(ast); + if (!f) { + const char *name = ast_strdupa(ast_channel_name(ast)); - /* tell the cdr this zap device hung up */ - if (f == NULL) { - ast_set_hangupsource(ast, ast_channel_name(ast), 0); + /* Tell the CDR this DAHDI device hung up */ + ast_mutex_unlock(&p->lock); + ast_channel_unlock(ast); + ast_set_hangupsource(ast, name, 0); + ast_channel_lock(ast); + ast_mutex_lock(&p->lock); } - return f; } diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c index ade8438bd..467cdb4e3 100644 --- a/channels/chan_iax2.c +++ b/channels/chan_iax2.c @@ -9898,11 +9898,20 @@ static void set_hangup_source_and_cause(int callno, unsigned char causecode) { iax2_lock_owner(callno); if (iaxs[callno] && iaxs[callno]->owner) { + struct ast_channel *owner; + const char *name; + + owner = iaxs[callno]->owner; if (causecode) { - ast_channel_hangupcause_set(iaxs[callno]->owner, causecode); + ast_channel_hangupcause_set(owner, causecode); } - ast_set_hangupsource(iaxs[callno]->owner, ast_channel_name(iaxs[callno]->owner), 0); - ast_channel_unlock(iaxs[callno]->owner); + name = ast_strdupa(ast_channel_name(owner)); + ast_channel_ref(owner); + ast_channel_unlock(owner); + ast_mutex_unlock(&iaxsl[callno]); + ast_set_hangupsource(owner, name, 0); + ast_channel_unref(owner); + ast_mutex_lock(&iaxsl[callno]); } } diff --git a/channels/chan_sip.c b/channels/chan_sip.c index d8c9f3958..934f8a611 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -20973,6 +20973,41 @@ static void handle_response_publish(struct sip_pvt *p, int resp, const char *res } } +/*! + * \internal + * \brief Set hangup source and cause. + * + * \param p SIP private. + * \param cause Hangup cause to queue. Zero if no cause. + * + * \pre p and p->owner are locked. + * + * \return Nothing + */ +static void sip_queue_hangup_cause(struct sip_pvt *p, int cause) +{ + struct ast_channel *owner = p->owner; + const char *name = ast_strdupa(ast_channel_name(owner)); + + /* Cannot hold any channel/private locks when calling. */ + ast_channel_ref(owner); + ast_channel_unlock(owner); + sip_pvt_unlock(p); + ast_set_hangupsource(owner, name, 0); + if (cause) { + ast_queue_hangup_with_cause(owner, cause); + } else { + ast_queue_hangup(owner); + } + ast_channel_unref(owner); + + /* Relock things. */ + owner = sip_pvt_lock_full(p); + if (owner) { + ast_channel_unref(owner); + } +} + /*! \brief Handle SIP response to INVITE dialogue */ static void handle_response_invite(struct sip_pvt *p, int resp, const char *rest, struct sip_request *req, uint32_t seqno) { @@ -21339,16 +21374,14 @@ static void handle_response_invite(struct sip_pvt *p, int resp, const char *rest xmitres = transmit_request(p, SIP_ACK, seqno, XMIT_UNRELIABLE, FALSE); ast_log(LOG_WARNING, "Received response: \"Forbidden\" from '%s'\n", sip_get_header(&p->initreq, "From")); if (!req->ignore && p->owner) { - ast_set_hangupsource(p->owner, ast_channel_name(p->owner), 0); - ast_queue_hangup_with_cause(p->owner, hangup_sip2cause(resp)); + sip_queue_hangup_cause(p, hangup_sip2cause(resp)); } break; case 404: /* Not found */ xmitres = transmit_request(p, SIP_ACK, seqno, XMIT_UNRELIABLE, FALSE); if (p->owner && !req->ignore) { - ast_set_hangupsource(p->owner, ast_channel_name(p->owner), 0); - ast_queue_hangup_with_cause(p->owner, hangup_sip2cause(resp)); + sip_queue_hangup_cause(p, hangup_sip2cause(resp)); } break; @@ -24882,11 +24915,10 @@ static int handle_request_cancel(struct sip_pvt *p, struct sip_request *req) stop_media_flows(p); /* Immediately stop RTP, VRTP and UDPTL as applicable */ if (p->owner) { - ast_set_hangupsource(p->owner, ast_channel_name(p->owner), 0); - ast_queue_hangup(p->owner); - } - else + sip_queue_hangup_cause(p, 0); + } else { sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); + } if (ast_str_strlen(p->initreq.data) > 0) { struct sip_pkt *pkt, *prev_pkt; /* If the CANCEL we are receiving is a retransmission, and we already have scheduled @@ -25040,8 +25072,7 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req) ast_queue_hangup_with_cause(p->owner, AST_CAUSE_PROTOCOL_ERROR); } } else if (p->owner) { - ast_set_hangupsource(p->owner, ast_channel_name(p->owner), 0); - ast_queue_hangup(p->owner); + sip_queue_hangup_cause(p, 0); sip_scheddestroy_final(p, DEFAULT_TRANS_TIMEOUT); ast_debug(3, "Received bye, issuing owner hangup\n"); } else { diff --git a/channels/sig_analog.c b/channels/sig_analog.c index 5d71696be..cabb16472 100644 --- a/channels/sig_analog.c +++ b/channels/sig_analog.c @@ -3623,7 +3623,18 @@ struct ast_frame *analog_exception(struct analog_pvt *p, struct ast_channel *ast f = &p->subs[idx].f; return f; } + f = __analog_handle_event(p, ast); + if (!f) { + const char *name = ast_strdupa(ast_channel_name(ast)); + + /* Tell the CDR this DAHDI device hung up */ + analog_unlock_private(p); + ast_channel_unlock(ast); + ast_set_hangupsource(ast, name, 0); + ast_channel_lock(ast); + analog_lock_private(p); + } return f; } diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index f183fe00e..170755b79 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -1388,6 +1388,8 @@ void ast_channel_clear_softhangup(struct ast_channel *chan, int flag); * \param source a string describing the source of the hangup for this channel * \param force * + * \note Absolutely _NO_ channel locks should be held before calling this function. + * * \since 1.8 * * Hangupsource is generally the channel name that caused the bridge to be diff --git a/main/channel.c b/main/channel.c index bfca3ec8d..6b3621dae 100644 --- a/main/channel.c +++ b/main/channel.c @@ -2566,12 +2566,18 @@ void ast_set_hangupsource(struct ast_channel *chan, const char *source, int forc ast_channel_hangupsource_set(chan, source); } bridge = ast_bridged_channel(chan); + if (bridge) { + ast_channel_ref(bridge); + } ast_channel_unlock(chan); - if (bridge && (force || ast_strlen_zero(ast_channel_hangupsource(bridge)))) { + if (bridge) { ast_channel_lock(bridge); - ast_channel_hangupsource_set(chan, source); + if (force || ast_strlen_zero(ast_channel_hangupsource(bridge))) { + ast_channel_hangupsource_set(bridge, source); + } ast_channel_unlock(bridge); + ast_channel_unref(bridge); } } |