Fix crash during CDR update.

The ast_cdr_setcid() and ast_cdr_update() were shown in ASTERISK-18836 to
be called by different threads for the same channel.  The channel driver
thread and the PBX thread running dialplan.

* Add lock protection around CDR API calls that access an ast_channel
pointer.

(closes issue ASTERISK-18836)
Reported by: gpluser

Review: https://reviewboard.asterisk.org/r/1628/
........

Merged revisions 348362 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 348363 from http://svn.asterisk.org/svn/asterisk/branches/10


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@348364 65c4cc65-6c06-0410-ace0-fbb531ad65f3

3 files changed, 38 insertions, 10 deletions

diff --git a/main/channel.c b/main/channel.c
index bb1bd4e7c..57788b5bd 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -5435,7 +5435,9 @@ struct ast_channel *ast_call_forward(struct ast_channel *caller, struct ast_chan
 			ast_channel_datastore_inherit(oh->parent_channel, new);
 		}
 		if (oh->account) {
+			ast_channel_lock(new);
 			ast_cdr_setaccount(new, oh->account);
+			ast_channel_lock(new);
 		}
 	} else if (caller) { /* no outgoing helper so use caller if avaliable */
 		ast_channel_update_redirecting(caller, apr, NULL);
@@ -5501,8 +5503,11 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c
 			ast_channel_inherit_variables(oh->parent_channel, chan);
 			ast_channel_datastore_inherit(oh->parent_channel, chan);
 		}
-		if (oh->account)
-			ast_cdr_setaccount(chan, oh->account);	
+		if (oh->account) {
+			ast_channel_lock(chan);
+			ast_cdr_setaccount(chan, oh->account);
+			ast_channel_unlock(chan);
+		}
 	}
 
 	ast_set_callerid(chan, cid_num, cid_name, cid_num);
@@ -5614,21 +5619,27 @@ struct ast_channel *__ast_request_and_dial(const char *type, struct ast_format_c
 		*outstate = AST_CONTROL_ANSWER;
 
 	if (res <= 0) {
-		if ( AST_CONTROL_RINGING == last_subclass ) 
+		ast_channel_lock(chan);
+		if (AST_CONTROL_RINGING == last_subclass) {
 			chan->hangupcause = AST_CAUSE_NO_ANSWER;
-		if (!chan->cdr && (chan->cdr = ast_cdr_alloc()))
+		}
+		if (!chan->cdr && (chan->cdr = ast_cdr_alloc())) {
 			ast_cdr_init(chan->cdr, chan);
+		}
 		if (chan->cdr) {
 			char tmp[256];
+
 			snprintf(tmp, sizeof(tmp), "%s/%s", type, (char *)data);
-			ast_cdr_setapp(chan->cdr,"Dial",tmp);
+			ast_cdr_setapp(chan->cdr, "Dial", tmp);
 			ast_cdr_update(chan);
 			ast_cdr_start(chan->cdr);
 			ast_cdr_end(chan->cdr);
 			/* If the cause wasn't handled properly */
-			if (ast_cdr_disposition(chan->cdr,chan->hangupcause))
+			if (ast_cdr_disposition(chan->cdr, chan->hangupcause)) {
 				ast_cdr_failed(chan->cdr);
+			}
 		}
+		ast_channel_unlock(chan);
 		ast_hangup(chan);
 		chan = NULL;
 	}
diff --git a/main/features.c b/main/features.c
index 6f23bd357..bb925f73d 100644
--- a/main/features.c
+++ b/main/features.c
@@ -2303,6 +2303,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
 	pbx_builtin_setvar_helper(transferer, "BLINDTRANSFER", transferee->name);
 	pbx_builtin_setvar_helper(transferee, "BLINDTRANSFER", transferer->name);
 	finishup(transferee);
+	ast_channel_lock(transferer);
 	if (!transferer->cdr) { /* this code should never get called (in a perfect world) */
 		transferer->cdr = ast_cdr_alloc();
 		if (transferer->cdr) {
@@ -2310,6 +2311,7 @@ static int builtin_blindtransfer(struct ast_channel *chan, struct ast_channel *p
 			ast_cdr_start(transferer->cdr);
 		}
 	}
+	ast_channel_unlock(transferer);
 	if (transferer->cdr) {
 		struct ast_cdr *swap = transferer->cdr;
 
@@ -3911,11 +3913,15 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
 	/* copy the userfield from the B-leg to A-leg if applicable */
 	if (chan->cdr && peer->cdr && !ast_strlen_zero(peer->cdr->userfield)) {
 		char tmp[256];
+
+		ast_channel_lock(chan);
 		if (!ast_strlen_zero(chan->cdr->userfield)) {
 			snprintf(tmp, sizeof(tmp), "%s;%s", chan->cdr->userfield, peer->cdr->userfield);
 			ast_cdr_appenduserfield(chan, tmp);
-		} else
+		} else {
 			ast_cdr_setuserfield(chan, peer->cdr->userfield);
+		}
+		ast_channel_unlock(chan);
 		/* Don't delete the CDR; just disable it. */
 		ast_set_flag(peer->cdr, AST_CDR_FLAG_POST_DISABLED);
 		we_disabled_peer_cdr = 1;
@@ -3924,7 +3930,7 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
 	ast_copy_string(orig_peername,peer->name,sizeof(orig_peername));
 
 	if (!chan_cdr || (chan_cdr && !ast_test_flag(chan_cdr, AST_CDR_FLAG_POST_DISABLED))) {
-		
+		ast_channel_lock_both(chan, peer);
 		if (chan_cdr) {
 			ast_set_flag(chan_cdr, AST_CDR_FLAG_MAIN);
 			ast_cdr_update(chan);
@@ -3939,7 +3945,6 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
 				ast_copy_string(bridge_cdr->userfield, peer_cdr->userfield, sizeof(bridge_cdr->userfield));
 			}
 			ast_cdr_setaccount(peer, chan->accountcode);
-
 		} else {
 			/* better yet, in a xfer situation, find out why the chan cdr got zapped (pun unintentional) */
 			bridge_cdr = ast_cdr_alloc(); /* this should be really, really rare/impossible? */
@@ -3962,6 +3967,9 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
 				ast_cdr_start(bridge_cdr);
 			}
 		}
+		ast_channel_unlock(chan);
+		ast_channel_unlock(peer);
+
 		ast_debug(4, "bridge answer set, chan answer set\n");
 		/* peer_cdr->answer will be set when a macro runs on the peer;
 		   in that case, the bridge answer will be delayed while the
diff --git a/main/pbx.c b/main/pbx.c
index fc4b3d93a..3ae294fcc 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -5112,10 +5112,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
 		set_ext_pri(c, "s", 1);
 	}
 
+	ast_channel_lock(c);
 	if (c->cdr) {
 		/* allow CDR variables that have been collected after channel was created to be visible during call */
 		ast_cdr_update(c);
 	}
+	ast_channel_unlock(c);
 	for (;;) {
 		char dst_exten[256];	/* buffer to accumulate digits */
 		int pos = 0;		/* XXX should check bounds */
@@ -5222,8 +5224,11 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
 					}
 					/* Call timed out with no special extension to jump to. */
 				}
-				if (c->cdr)
+				ast_channel_lock(c);
+				if (c->cdr) {
 					ast_cdr_update(c);
+				}
+				ast_channel_unlock(c);
 				error = 1;
 				break;
 			}
@@ -5327,10 +5332,12 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c,
 					}
 				}
 			}
+			ast_channel_lock(c);
 			if (c->cdr) {
 				ast_verb(2, "CDR updated on %s\n",c->name);
 				ast_cdr_update(c);
 			}
+			ast_channel_unlock(c);
 		}
 	}
 
@@ -9571,7 +9578,9 @@ static int pbx_builtin_resetcdr(struct ast_channel *chan, const char *data)
 static int pbx_builtin_setamaflags(struct ast_channel *chan, const char *data)
 {
 	/* Copy the AMA Flags as specified */
+	ast_channel_lock(chan);
 	ast_cdr_setamaflags(chan, data ? data : "");
+	ast_channel_unlock(chan);
 	return 0;
 }