[asterisk-commits] rmudgett: branch 1.8 r368759 - in /branches/1.8: channels/ include/asterisk/ ...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Jun 11 12:03:07 CDT 2012


Author: rmudgett
Date: Mon Jun 11 12:03:02 2012
New Revision: 368759

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=368759
Log:
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

Modified:
    branches/1.8/channels/chan_dahdi.c
    branches/1.8/channels/chan_iax2.c
    branches/1.8/channels/chan_sip.c
    branches/1.8/channels/sig_analog.c
    branches/1.8/include/asterisk/channel.h
    branches/1.8/main/channel.c

Modified: branches/1.8/channels/chan_dahdi.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_dahdi.c?view=diff&rev=368759&r1=368758&r2=368759
==============================================================================
--- branches/1.8/channels/chan_dahdi.c (original)
+++ branches/1.8/channels/chan_dahdi.c Mon Jun 11 12:03:02 2012
@@ -8870,13 +8870,18 @@
 		f = &p->subs[idx].f;
 		return f;
 	}
+
 	f = dahdi_handle_event(ast);
-
-	/* tell the cdr this zap device hung up */
-	if (f == NULL) {
-		ast_set_hangupsource(ast, ast->name, 0);
-	}
-
+	if (!f) {
+		const char *name = ast_strdupa(ast->name);
+
+		/* 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;
 }
 

Modified: branches/1.8/channels/chan_iax2.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_iax2.c?view=diff&rev=368759&r1=368758&r2=368759
==============================================================================
--- branches/1.8/channels/chan_iax2.c (original)
+++ branches/1.8/channels/chan_iax2.c Mon Jun 11 12:03:02 2012
@@ -9890,11 +9890,20 @@
 {
 	iax2_lock_owner(callno);
 	if (iaxs[callno] && iaxs[callno]->owner) {
+		struct ast_channel *owner;
+		const char *name;
+
+		owner = iaxs[callno]->owner;
 		if (causecode) {
-			iaxs[callno]->owner->hangupcause = causecode;
-		}
-		ast_set_hangupsource(iaxs[callno]->owner, iaxs[callno]->owner->name, 0);
-		ast_channel_unlock(iaxs[callno]->owner);
+			owner->hangupcause = causecode;
+		}
+		name = ast_strdupa(owner->name);
+		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]);
 	}
 }
 

Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=368759&r1=368758&r2=368759
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Mon Jun 11 12:03:02 2012
@@ -20026,6 +20026,41 @@
 	}
 }
 
+/*!
+ * \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(owner->name);
+
+	/* 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)
 {
@@ -20370,16 +20405,14 @@
 		xmitres = transmit_request(p, SIP_ACK, seqno, XMIT_UNRELIABLE, FALSE);
 		ast_log(LOG_WARNING, "Received response: \"Forbidden\" from '%s'\n", get_header(&p->initreq, "From"));
 		if (!req->ignore && p->owner) {
-			ast_set_hangupsource(p->owner, p->owner->name, 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, p->owner->name, 0);
-			ast_queue_hangup_with_cause(p->owner, hangup_sip2cause(resp));
+			sip_queue_hangup_cause(p, hangup_sip2cause(resp));
 		}
 		break;
 
@@ -23828,11 +23861,10 @@
 
 	stop_media_flows(p); /* Immediately stop RTP, VRTP and UDPTL as applicable */
 	if (p->owner) {
-		ast_set_hangupsource(p->owner, p->owner->name, 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
@@ -23986,8 +24018,7 @@
 				ast_queue_hangup_with_cause(p->owner, AST_CAUSE_PROTOCOL_ERROR);
 		}
 	} else if (p->owner) {
-		ast_set_hangupsource(p->owner, p->owner->name, 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 {

Modified: branches/1.8/channels/sig_analog.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/sig_analog.c?view=diff&rev=368759&r1=368758&r2=368759
==============================================================================
--- branches/1.8/channels/sig_analog.c (original)
+++ branches/1.8/channels/sig_analog.c Mon Jun 11 12:03:02 2012
@@ -3617,7 +3617,18 @@
 		f = &p->subs[idx].f;
 		return f;
 	}
+
 	f = __analog_handle_event(p, ast);
+	if (!f) {
+		const char *name = ast_strdupa(ast->name);
+
+		/* 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;
 }
 

Modified: branches/1.8/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/channel.h?view=diff&rev=368759&r1=368758&r2=368759
==============================================================================
--- branches/1.8/include/asterisk/channel.h (original)
+++ branches/1.8/include/asterisk/channel.h Mon Jun 11 12:03:02 2012
@@ -1470,6 +1470,8 @@
  * \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

Modified: branches/1.8/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/channel.c?view=diff&rev=368759&r1=368758&r2=368759
==============================================================================
--- branches/1.8/main/channel.c (original)
+++ branches/1.8/main/channel.c Mon Jun 11 12:03:02 2012
@@ -2731,12 +2731,18 @@
 		ast_string_field_set(chan, hangupsource, source);
 	}
 	bridge = ast_bridged_channel(chan);
+	if (bridge) {
+		ast_channel_ref(bridge);
+	}
 	ast_channel_unlock(chan);
 
-	if (bridge && (force || ast_strlen_zero(bridge->hangupsource))) {
+	if (bridge) {
 		ast_channel_lock(bridge);
-		ast_string_field_set(chan, hangupsource, source);
+		if (force || ast_strlen_zero(bridge->hangupsource)) {
+			ast_string_field_set(bridge, hangupsource, source);
+		}
 		ast_channel_unlock(bridge);
+		ast_channel_unref(bridge);
 	}
 }
 




More information about the asterisk-commits mailing list