From: Richard Mudgett <rmudgett@digium.com>
Date: Tue, 23 Aug 2016 20:57:58 +0000 (-0500)
Subject: res_fax.c, res_fax_spandsp.c: Misc deadlock fixes.
X-Git-Tag: 11.24.0-rc1~25
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2e7ad40f4181287fbc648ef21ba0a0a0229f64dc;p=thirdparty%2Fasterisk.git

res_fax.c, res_fax_spandsp.c: Misc deadlock fixes.

* Fixed locking inconsistency in fax_gateway_start() when calling the
gateway technology start_session callback.  The callback was called with
and without the channel lock held.  Calling the callback with the channel
lock held could deadlock in spandsp_fax_gateway_start() which calls
ast_channel_get_t38_state().

* Fixed deadlock potential in fax_detect_framehook() when calling
ast_channel_make_compatible().

ASTERISK-26203
Reported by: Etienne Lessard

ASTERISK-24822
Reported by: David Brillert

ASTERISK-22732
Reported by: Richard Mudgett

Change-Id: I5a7b8e9e7b198802df06dff0ea48d96bd6d7b912
---

diff --git a/res/res_fax.c b/res/res_fax.c
index 979b683505..83ca28280a 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -1365,7 +1365,7 @@ static int generic_fax_exec(struct ast_channel *chan, struct ast_fax_session_det
 	report_fax_status(chan, details, "Allocating Resources");
 
 	if (details->caps & AST_FAX_TECH_AUDIO) {
-		expected_frametype = AST_FRAME_VOICE;;
+		expected_frametype = AST_FRAME_VOICE;
 		ast_format_set(&expected_framesubclass.format, AST_FORMAT_SLINEAR, 0);
 		ast_format_copy(&orig_write_format, ast_channel_writeformat(chan));
 		if (ast_set_write_format_by_id(chan, AST_FORMAT_SLINEAR) < 0) {
@@ -2591,6 +2591,7 @@ static struct fax_gateway *fax_gateway_new(struct ast_channel *chan, struct ast_
 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))) {
@@ -2611,7 +2612,10 @@ static int fax_gateway_start(struct fax_gateway *gateway, struct ast_fax_session
 	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");
@@ -3316,8 +3320,12 @@ static struct ast_frame *fax_detect_framehook(struct ast_channel *chan, struct a
 	struct ast_fax_session_details *details;
 	struct ast_control_t38_parameters *control_params;
 	struct ast_channel *peer;
+	RAII_VAR(struct ast_channel *, chan_ref, chan, ao2_cleanup);
 	int result = 0;
 
+	/* Ref bump the channel for when we have to unlock it */
+	ao2_ref(chan, 1);
+
 	details = faxdetect->details;
 
 	switch (event) {
@@ -3340,8 +3348,13 @@ static struct ast_frame *fax_detect_framehook(struct ast_channel *chan, struct a
 	case AST_FRAMEHOOK_EVENT_DETACHED:
 		/* restore audio formats when we are detached */
 		ast_set_read_format(chan, &faxdetect->orig_format);
-		if ((peer = ast_bridged_channel(chan))) {
+		peer = ast_bridged_channel(chan);
+		if (peer) {
+			ao2_ref(peer, +1);
+			ast_channel_unlock(chan);
 			ast_channel_make_compatible(chan, peer);
+			ast_channel_lock(chan);
+			ao2_ref(peer, -1);
 		}
 		return NULL;
 	case AST_FRAMEHOOK_EVENT_READ:
diff --git a/res/res_fax_spandsp.c b/res/res_fax_spandsp.c
index d8b8af1d0f..5aa354cf93 100644
--- a/res/res_fax_spandsp.c
+++ b/res/res_fax_spandsp.c
@@ -831,10 +831,14 @@ static int spandsp_fax_gateway_start(struct ast_fax_session *s)
 
 	p->ist38 = 1;
 	p->ast_t38_state = ast_channel_get_t38_state(s->chan);
-	if (!(peer = ast_bridged_channel(s->chan))) {
+	ast_channel_lock(s->chan);
+	peer = ast_bridged_channel(s->chan);
+	if (!peer) {
 		ast_channel_unlock(s->chan);
 		return -1;
 	}
+	ao2_ref(peer, +1);
+	ast_channel_unlock(s->chan);
 
 	/* we can be in T38_STATE_NEGOTIATING or T38_STATE_NEGOTIATED when the
 	 * gateway is started. We treat both states the same. */
@@ -843,6 +847,7 @@ static int spandsp_fax_gateway_start(struct ast_fax_session *s)
 	}
 
 	ast_activate_generator(p->ast_t38_state == T38_STATE_NEGOTIATED ? peer : s->chan, &t30_gen , s);
+	ao2_ref(peer, -1);
 
 	set_logging(&p->t38_gw_state.logging, s->details);
 	set_logging(&p->t38_core_state->logging, s->details);