[Asterisk-code-review] features: Fix channel datastore access. (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: features: Fix channel datastore access.
......................................................................


features: Fix channel datastore access.

Found as a result of the testsuite tests/callparking test crashing.

Several calls to ast_get_chan_featuremap_config() and
ast_get_chan_features_xfer_config() did not lock the channel before
calling so the channel's datastore list was accessed without the lock's
protection.  Apparently another thread deleted a datastore on the
channel's list while the crashing thread was walking the list.  Crash at
0xdeaddead due to MALLOC_DEBUG's memory filler value as a result.

* Add missing channel locks to calls that were not already protected
as the doxygen for those calls indicates.

Change-Id: Id273b3d305cc616406c353cbc841b2b7655efaa1
---
M main/bridge_channel.c
M main/features.c
2 files changed, 11 insertions(+), 4 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve; Verified



diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 543988d..2fafdf9 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -1543,8 +1543,13 @@
 {
 #ifdef TEST_FRAMEWORK
 	char *feature = "unknown";
-	struct ast_featuremap_config *featuremap = ast_get_chan_featuremap_config(chan);
-	struct ast_features_xfer_config *xfer = ast_get_chan_features_xfer_config(chan);
+	struct ast_featuremap_config *featuremap;
+	struct ast_features_xfer_config *xfer;
+
+	ast_channel_lock(chan);
+	featuremap = ast_get_chan_featuremap_config(chan);
+	xfer = ast_get_chan_features_xfer_config(chan);
+	ast_channel_unlock(chan);
 
 	if (featuremap) {
 		if (!strcmp(dtmf, featuremap->blindxfer)) {
diff --git a/main/features.c b/main/features.c
index b96cbd6..0001051 100644
--- a/main/features.c
+++ b/main/features.c
@@ -767,8 +767,8 @@
 		astman_send_error(s, m, buf);
 		return 0;
 	}
-	xfer_cfg_a = ast_get_chan_features_xfer_config(chana);
 	ast_channel_lock(chana);
+	xfer_cfg_a = ast_get_chan_features_xfer_config(chana);
 	chana_exten = ast_strdupa(ast_channel_exten(chana));
 	chana_context = ast_strdupa(ast_channel_context(chana));
 	chana_priority = ast_channel_priority(chana);
@@ -783,8 +783,8 @@
 		astman_send_error(s, m, buf);
 		return 0;
 	}
-	xfer_cfg_b = ast_get_chan_features_xfer_config(chanb);
 	ast_channel_lock(chanb);
+	xfer_cfg_b = ast_get_chan_features_xfer_config(chanb);
 	chanb_exten = ast_strdupa(ast_channel_exten(chanb));
 	chanb_context = ast_strdupa(ast_channel_context(chanb));
 	chanb_priority = ast_channel_priority(chanb);
@@ -1098,7 +1098,9 @@
 		goto done;
 	}
 
+	ast_channel_lock(current_dest_chan);
 	xfer_cfg = ast_get_chan_features_xfer_config(current_dest_chan);
+	ast_channel_unlock(current_dest_chan);
 	bridge_add_failed = ast_bridge_add_channel(bridge, current_dest_chan, peer_features,
 		ast_test_flag(&opts, BRIDGE_OPT_PLAYTONE),
 		xfer_cfg ? xfer_cfg->xfersound : NULL);

-- 
To view, visit https://gerrit.asterisk.org/3124
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id273b3d305cc616406c353cbc841b2b7655efaa1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>