[Asterisk-code-review] features: Fix crash when transferee hangs up during DTMF att... (asterisk[master])
Richard Mudgett
asteriskteam at digium.com
Tue May 5 18:23:38 CDT 2015
Richard Mudgett has uploaded a new change for review.
https://gerrit.asterisk.org/373
Change subject: features: Fix crash when transferee hangs up during DTMF attended transfer.
......................................................................
features: Fix crash when transferee hangs up during DTMF attended transfer.
A crash happens with this sequence of steps:
1) Party A is connected to party B.
2) Party B starts a DTMF attended transfer.
3) Party A hangs up while party B is dialing party C.
When party A hangs up the bridge that party A and party B are in is
dissolved and party B is kicked out of the bridge. When party B finishes
dialing party C he attempts to move to the new bridge with party C. Since
party B is no longer in a bridge the attempted move dereferences a NULL
bridge_channel pointer and crashes.
* Made the hold(), unhold(), ringing(), and the bridge_move() functions
tolerant of the channel not being in a bridge. The assertion that party B
is always in a bridge is not true if the bridged peer of party B hangs up
and dissolves the bridge. Being tolerant of not being in a bridge allows
the peer hangup stimulus to be processed by the FSM.
* Made the bridge_move() function return void since where the return value
for a failed move was checked generated a FSM coding ERROR message for a
normal off-nominal condition.
* Eliminated most uses of RAII_VAR in bridge_basic.c.
ASTERISK-25003 #close
Reported by: Artem Volodin
Change-Id: Ie2c1b14e5e647d4ea6de300bf56d69805d7bcada
---
M main/bridge_basic.c
1 file changed, 67 insertions(+), 58 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/73/373/1
diff --git a/main/bridge_basic.c b/main/bridge_basic.c
index c48db5c..5e20f30 100644
--- a/main/bridge_basic.c
+++ b/main/bridge_basic.c
@@ -606,17 +606,16 @@
*/
static int setup_bridge_features_dynamic(struct ast_bridge_features *features, struct ast_channel *chan)
{
- RAII_VAR(struct ao2_container *, applicationmap, NULL, ao2_cleanup);
+ struct ao2_container *applicationmap;
int res = 0;
ast_channel_lock(chan);
applicationmap = ast_get_chan_applicationmap(chan);
ast_channel_unlock(chan);
- if (!applicationmap) {
- return 0;
+ if (applicationmap) {
+ ao2_callback_data(applicationmap, 0, setup_dynamic_feature, features, &res);
+ ao2_ref(applicationmap, -1);
}
-
- ao2_callback_data(applicationmap, 0, setup_dynamic_feature, features, &res);
return res;
}
@@ -1418,7 +1417,7 @@
char *tech;
char *addr;
char *serial;
- RAII_VAR(struct ast_features_xfer_config *, xfer_cfg, NULL, ao2_cleanup);
+ struct ast_features_xfer_config *xfer_cfg;
struct ast_flags *transferer_features;
props = ao2_alloc(sizeof(*props), attended_transfer_properties_destructor);
@@ -1450,6 +1449,7 @@
ast_string_field_set(props, context, get_transfer_context(transferer, context));
ast_string_field_set(props, failsound, xfer_cfg->xferfailsound);
ast_string_field_set(props, xfersound, xfer_cfg->xfersound);
+ ao2_ref(xfer_cfg, -1);
/*
* Save the transferee's party information for any recall calls.
@@ -1695,17 +1695,16 @@
*/
static void play_sound(struct ast_channel *chan, const char *sound)
{
- RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *bridge_channel;
ast_channel_lock(chan);
bridge_channel = ast_channel_get_bridge_channel(chan);
ast_channel_unlock(chan);
- if (!bridge_channel) {
- return;
+ if (bridge_channel) {
+ ast_bridge_channel_queue_playfile(bridge_channel, NULL, sound, NULL);
+ ao2_ref(bridge_channel, -1);
}
-
- ast_bridge_channel_queue_playfile(bridge_channel, NULL, sound, NULL);
}
/*!
@@ -1713,16 +1712,19 @@
*/
static void hold(struct ast_channel *chan)
{
- RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *bridge_channel;
- if (chan) {
- ast_channel_lock(chan);
- bridge_channel = ast_channel_get_bridge_channel(chan);
- ast_channel_unlock(chan);
+ if (!chan) {
+ return;
+ }
- ast_assert(bridge_channel != NULL);
+ ast_channel_lock(chan);
+ bridge_channel = ast_channel_get_bridge_channel(chan);
+ ast_channel_unlock(chan);
+ if (bridge_channel) {
ast_bridge_channel_write_hold(bridge_channel, NULL);
+ ao2_ref(bridge_channel, -1);
}
}
@@ -1731,15 +1733,20 @@
*/
static void unhold(struct ast_channel *chan)
{
- RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *bridge_channel;
+
+ if (!chan) {
+ return;
+ }
ast_channel_lock(chan);
bridge_channel = ast_channel_get_bridge_channel(chan);
ast_channel_unlock(chan);
- ast_assert(bridge_channel != NULL);
-
- ast_bridge_channel_write_unhold(bridge_channel);
+ if (bridge_channel) {
+ ast_bridge_channel_write_unhold(bridge_channel);
+ ao2_ref(bridge_channel, -1);
+ }
}
/*!
@@ -1747,15 +1754,16 @@
*/
static void ringing(struct ast_channel *chan)
{
- RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *bridge_channel;
ast_channel_lock(chan);
bridge_channel = ast_channel_get_bridge_channel(chan);
ast_channel_unlock(chan);
- ast_assert(bridge_channel != NULL);
-
- ast_bridge_channel_write_control_data(bridge_channel, AST_CONTROL_RINGING, NULL, 0);
+ if (bridge_channel) {
+ ast_bridge_channel_write_control_data(bridge_channel, AST_CONTROL_RINGING, NULL, 0);
+ ao2_ref(bridge_channel, -1);
+ }
}
/*!
@@ -1800,10 +1808,9 @@
/*!
* \brief Wrapper for \ref bridge_do_move
*/
-static int bridge_move(struct ast_bridge *dest, struct ast_bridge *src, struct ast_channel *channel, struct ast_channel *swap)
+static void bridge_move(struct ast_bridge *dest, struct ast_bridge *src, struct ast_channel *channel, struct ast_channel *swap)
{
- int res;
- RAII_VAR(struct ast_bridge_channel *, bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *bridge_channel;
ast_bridge_lock_both(src, dest);
@@ -1811,18 +1818,18 @@
bridge_channel = ast_channel_get_bridge_channel(channel);
ast_channel_unlock(channel);
- ast_assert(bridge_channel != NULL);
+ if (bridge_channel) {
+ ao2_lock(bridge_channel);
+ bridge_channel->swap = swap;
+ ao2_unlock(bridge_channel);
- ao2_lock(bridge_channel);
- bridge_channel->swap = swap;
- ao2_unlock(bridge_channel);
-
- res = bridge_do_move(dest, bridge_channel, 1, 0);
+ bridge_do_move(dest, bridge_channel, 1, 0);
+ }
ast_bridge_unlock(dest);
ast_bridge_unlock(src);
- return res;
+ ao2_cleanup(bridge_channel);
}
/*!
@@ -2033,7 +2040,8 @@
static int calling_target_enter(struct attended_transfer_properties *props)
{
- return bridge_move(props->target_bridge, props->transferee_bridge, props->transferer, NULL);
+ bridge_move(props->target_bridge, props->transferee_bridge, props->transferer, NULL);
+ return 0;
}
static enum attended_transfer_state calling_target_exit(struct attended_transfer_properties *props,
@@ -2072,10 +2080,7 @@
static int hesitant_enter(struct attended_transfer_properties *props)
{
- if (bridge_move(props->transferee_bridge, props->target_bridge, props->transferer, NULL)) {
- return -1;
- }
-
+ bridge_move(props->transferee_bridge, props->target_bridge, props->transferer, NULL);
unhold(props->transferer);
return 0;
}
@@ -2115,11 +2120,7 @@
static int rebridge_enter(struct attended_transfer_properties *props)
{
- if (bridge_move(props->transferee_bridge, props->target_bridge,
- props->transferer, NULL)) {
- return -1;
- }
-
+ bridge_move(props->transferee_bridge, props->target_bridge, props->transferer, NULL);
unhold(props->transferer);
return 0;
}
@@ -2839,7 +2840,8 @@
}
if (self->num_channels == 1) {
- RAII_VAR(struct ast_bridge_channel *, transferer_bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *transferer_bridge_channel;
+ int not_transferer;
ast_channel_lock(props->transferer);
transferer_bridge_channel = ast_channel_get_bridge_channel(props->transferer);
@@ -2849,7 +2851,9 @@
return;
}
- if (AST_LIST_FIRST(&self->channels) != transferer_bridge_channel) {
+ not_transferer = AST_LIST_FIRST(&self->channels) != transferer_bridge_channel;
+ ao2_ref(transferer_bridge_channel, -1);
+ if (not_transferer) {
return;
}
}
@@ -2886,7 +2890,8 @@
}
if (self->num_channels == 1) {
- RAII_VAR(struct ast_bridge_channel *, target_bridge_channel, NULL, ao2_cleanup);
+ struct ast_bridge_channel *target_bridge_channel;
+
if (!props->recall_target) {
/* No recall target means that the pull happened on a transferee. If there's still
* a channel left in the bridge, we don't need to send a stimulus
@@ -2898,12 +2903,11 @@
target_bridge_channel = ast_channel_get_bridge_channel(props->recall_target);
ast_channel_unlock(props->recall_target);
- if (!target_bridge_channel) {
- return;
- }
-
- if (AST_LIST_FIRST(&self->channels) == target_bridge_channel) {
- stimulate_attended_transfer(props, STIMULUS_TRANSFEREE_HANGUP);
+ if (target_bridge_channel) {
+ if (AST_LIST_FIRST(&self->channels) == target_bridge_channel) {
+ stimulate_attended_transfer(props, STIMULUS_TRANSFEREE_HANGUP);
+ }
+ ao2_ref(target_bridge_channel, -1);
}
}
}
@@ -2925,7 +2929,8 @@
static enum attended_transfer_stimulus wait_for_stimulus(struct attended_transfer_properties *props)
{
- RAII_VAR(struct stimulus_list *, list, NULL, ast_free_ptr);
+ enum attended_transfer_stimulus stimulus;
+ struct stimulus_list *list;
SCOPED_MUTEX(lock, ao2_object_get_lockaddr(props));
while (!(list = AST_LIST_REMOVE_HEAD(&props->stimulus_queue, next))) {
@@ -2956,7 +2961,9 @@
}
}
}
- return list->stimulus;
+ stimulus = list->stimulus;
+ ast_free(list);
+ return stimulus;
}
/*!
@@ -3050,7 +3057,7 @@
const char *atxfer_threeway;
const char *atxfer_complete;
const char *atxfer_swap;
- RAII_VAR(struct ast_features_xfer_config *, xfer_cfg, NULL, ao2_cleanup);
+ struct ast_features_xfer_config *xfer_cfg;
SCOPED_CHANNELLOCK(lock, chan);
xfer_cfg = ast_get_chan_features_xfer_config(chan);
@@ -3068,6 +3075,7 @@
atxfer_complete = ast_strdupa(xfer_cfg->atxfercomplete);
atxfer_swap = ast_strdupa(xfer_cfg->atxferswap);
}
+ ao2_ref(xfer_cfg, -1);
return ast_channel_add_bridge_role(chan, AST_TRANSFERER_ROLE_NAME) ||
ast_channel_set_bridge_role_option(chan, AST_TRANSFERER_ROLE_NAME, "abort", atxfer_abort) ||
@@ -3088,7 +3096,7 @@
int digit_timeout;
int attempts = 0;
int max_attempts;
- RAII_VAR(struct ast_features_xfer_config *, xfer_cfg, NULL, ao2_cleanup);
+ struct ast_features_xfer_config *xfer_cfg;
char *retry_sound;
char *invalid_sound;
@@ -3103,6 +3111,7 @@
max_attempts = xfer_cfg->transferdialattempts;
retry_sound = ast_strdupa(xfer_cfg->transferretrysound);
invalid_sound = ast_strdupa(xfer_cfg->transferinvalidsound);
+ ao2_ref(xfer_cfg, -1);
ast_channel_unlock(chan);
/* Play the simple "transfer" prompt out and wait */
--
To view, visit https://gerrit.asterisk.org/373
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2c1b14e5e647d4ea6de300bf56d69805d7bcada
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list