From a63656b41975a4f77c8d953b5dc7f966332892f4 Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Wed, 13 Apr 2016 13:50:04 -0500
Subject: [PATCH] Bridge system: Fix memory leaks and double frees on impart
failure.
You cannot reference the passed in features struct after calling
ast_bridge_impart(). Even if the call fails.
Change-Id: I902b88ba0d5d39520e670fb635078a367268ea21
---
apps/confbridge/conf_chan_announce.c | 1 -
include/asterisk/features.h | 9 ++++++++-
main/bridge.c | 7 ++++---
main/core_unreal.c | 1 -
main/features.c | 1 -
5 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/apps/confbridge/conf_chan_announce.c b/apps/confbridge/conf_chan_announce.c
index 8ced0cccd8..ff30499086 100644
--- a/apps/confbridge/conf_chan_announce.c
+++ b/apps/confbridge/conf_chan_announce.c
@@ -199,7 +199,6 @@ int conf_announce_channel_push(struct ast_channel *ast)
/* Impart the output channel into the bridge */
if (ast_bridge_impart(p->bridge, chan, NULL, features,
AST_BRIDGE_IMPART_CHAN_DEPARTABLE)) {
- ast_bridge_features_destroy(features);
ast_channel_unref(chan);
return -1;
}
diff --git a/include/asterisk/features.h b/include/asterisk/features.h
index b63124c2f1..a4aed5d18c 100644
--- a/include/asterisk/features.h
+++ b/include/asterisk/features.h
@@ -51,6 +51,7 @@ int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct a
/*!
* \brief Bridge a call, and add additional flags to the bridge
*
+ * \details
* This does the same thing as \ref ast_bridge_call, except that once the bridge
* is created, the provided flags are set on the bridge. The provided flags are
* added to the bridge's flags; they will not clear any flags already set.
@@ -70,6 +71,7 @@ int ast_bridge_call_with_flags(struct ast_channel *chan, struct ast_channel *pee
* \brief Add an arbitrary channel to a bridge
* \since 12.0.0
*
+ * \details
* The channel that is being added to the bridge can be in any state: unbridged,
* bridged, answered, unanswered, etc. The channel will be added asynchronously,
* meaning that when this function returns once the channel has been added to
@@ -87,11 +89,16 @@ int ast_bridge_call_with_flags(struct ast_channel *chan, struct ast_channel *pee
* \param features Features for this channel in the bridge
* \param play_tone Indicates if a tone should be played to the channel
* \param xfersound Sound that should be used to indicate transfer with play_tone
+ *
+ * \note The features parameter must be NULL or obtained by
+ * ast_bridge_features_new(). You must not dereference features
+ * after calling even if the call fails.
+ *
* \retval 0 Success
* \retval -1 Failure
*/
int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
- struct ast_bridge_features *features, int play_tone, const char *xfersound);
+ struct ast_bridge_features *features, int play_tone, const char *xfersound);
diff --git a/main/bridge.c b/main/bridge.c
index a56555bc9f..64c750b1e6 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -2318,6 +2318,9 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
if (chan_bridge) {
struct ast_bridge_channel *bridge_channel;
+ /* The channel is in a bridge so it is not getting any new features. */
+ ast_bridge_features_destroy(features);
+
ast_bridge_lock_both(bridge, chan_bridge);
bridge_channel = bridge_find_channel(chan_bridge, chan);
@@ -2340,9 +2343,6 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
bridge_dissolve_check_stolen(chan_bridge, bridge_channel);
ast_bridge_unlock(chan_bridge);
ast_bridge_unlock(bridge);
-
- /* The channel was in a bridge so it is not getting any new features. */
- ast_bridge_features_destroy(features);
} else {
/* Slightly less easy case. We need to yank channel A from
* where he currently is and impart him into our bridge.
@@ -2350,6 +2350,7 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
yanked_chan = ast_channel_yank(chan);
if (!yanked_chan) {
ast_log(LOG_WARNING, "Could not gain control of channel %s\n", ast_channel_name(chan));
+ ast_bridge_features_destroy(features);
return -1;
}
if (ast_channel_state(yanked_chan) != AST_STATE_UP) {
diff --git a/main/core_unreal.c b/main/core_unreal.c
index 377abd1ffa..28886b6359 100644
--- a/main/core_unreal.c
+++ b/main/core_unreal.c
@@ -805,7 +805,6 @@ int ast_unreal_channel_push_to_bridge(struct ast_channel *ast, struct ast_bridge
/* Impart the semi2 channel into the bridge */
if (ast_bridge_impart(bridge, chan, NULL, features,
AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
- ast_bridge_features_destroy(features);
ast_channel_unref(chan);
return -1;
}
diff --git a/main/features.c b/main/features.c
index 618f91c52d..7dfe4cde57 100644
--- a/main/features.c
+++ b/main/features.c
@@ -1103,7 +1103,6 @@ static int bridge_exec(struct ast_channel *chan, const char *data)
xfer_cfg ? xfer_cfg->xfersound : NULL);
ao2_cleanup(xfer_cfg);
if (bridge_add_failed) {
- ast_bridge_features_destroy(peer_features);
ast_bridge_features_cleanup(&chan_features);
ast_bridge_destroy(bridge, 0);
goto done;
--
GitLab