autochan/mixmonitor/chanspy: Fix unsafe channel locking and references.
Dereferencing struct ast_autochan.chan without first calling ast_autochan_channel_lock() is unsafe because the pointer could change at any time due to a masquerade. Unfortunately, ast_autochan_channel_lock() itself uses struct ast_autochan.chan unsafely and can result in a deadlock if the original channel happens to get destroyed after a masquerade in addition to the pointer getting changed. The problem is more likely to happen with v11 and earlier because masquerades are used to optimize out local channels on those versions. However, it could still happen on newer versions if the channel is executing a dialplan application when the channel is transferred or redirected. In this situation a masquerade still must be used. * Added a lock to struct ast_autochan to safely be able to use ast_autochan.chan while trying to get the channel lock in ast_autochan_channel_lock(). The locking order is the channel lock then the autochan lock. Locking in the other direction requires deadlock avoidance. * Fix unsafe ast_autochan.chan usages in app_mixmonitor.c. * Fix unsafe ast_autochan.chan usages in app_chanspy.c. * app_chanspy.c: Removed unused autochan parameter from next_channel(). ASTERISK-26867 Change-Id: Id29dd22bc0f369b44e23ca423d2f3657187cc592
This commit is contained in:
Richard Mudgett
parent
3fe1d8afba
commit
c87e7dd9ec
|
|
@ -506,18 +506,24 @@ static struct ast_generator spygen = {
|
|||
|
||||
static int start_spying(struct ast_autochan *autochan, const char *spychan_name, struct ast_audiohook *audiohook, struct ast_flags *flags)
|
||||
{
|
||||
int res;
|
||||
|
||||
ast_autochan_channel_lock(autochan);
|
||||
ast_log(LOG_NOTICE, "Attaching %s to %s\n", spychan_name, ast_channel_name(autochan->chan));
|
||||
if(ast_test_flag(flags, OPTION_READONLY)) {
|
||||
|
||||
if (ast_test_flag(flags, OPTION_READONLY)) {
|
||||
ast_set_flag(audiohook, AST_AUDIOHOOK_MUTE_WRITE);
|
||||
} else {
|
||||
ast_set_flag(audiohook, AST_AUDIOHOOK_TRIGGER_SYNC);
|
||||
}
|
||||
if(ast_test_flag(flags, OPTION_LONG_QUEUE)) {
|
||||
if (ast_test_flag(flags, OPTION_LONG_QUEUE)) {
|
||||
ast_debug(9, "Using a long queue to store audio frames in spy audiohook\n");
|
||||
} else {
|
||||
ast_set_flag(audiohook, AST_AUDIOHOOK_SMALL_QUEUE);
|
||||
}
|
||||
return ast_audiohook_attach(autochan->chan, audiohook);
|
||||
res = ast_audiohook_attach(autochan->chan, audiohook);
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
return res;
|
||||
}
|
||||
|
||||
static void change_spy_mode(const char digit, struct ast_flags *flags)
|
||||
|
|
@ -601,8 +607,14 @@ static int attach_barge(struct ast_autochan *spyee_autochan,
|
|||
{
|
||||
int retval = 0;
|
||||
struct ast_autochan *internal_bridge_autochan;
|
||||
RAII_VAR(struct ast_channel *, bridged, ast_channel_bridge_peer(spyee_autochan->chan), ast_channel_cleanup);
|
||||
struct ast_channel *spyee_chan;
|
||||
RAII_VAR(struct ast_channel *, bridged, NULL, ast_channel_cleanup);
|
||||
|
||||
ast_autochan_channel_lock(spyee_autochan);
|
||||
spyee_chan = ast_channel_ref(spyee_autochan->chan);
|
||||
ast_autochan_channel_unlock(spyee_autochan);
|
||||
bridged = ast_channel_bridge_peer(spyee_chan);
|
||||
ast_channel_unref(spyee_chan);
|
||||
if (!bridged) {
|
||||
return -1;
|
||||
}
|
||||
|
|
@ -614,12 +626,10 @@ static int attach_barge(struct ast_autochan *spyee_autochan,
|
|||
return -1;
|
||||
}
|
||||
|
||||
ast_autochan_channel_lock(internal_bridge_autochan);
|
||||
if (start_spying(internal_bridge_autochan, spyer_name, bridge_whisper_audiohook, flags)) {
|
||||
ast_log(LOG_WARNING, "Unable to attach barge audiohook on spyee '%s'. Barge mode disabled.\n", name);
|
||||
retval = -1;
|
||||
}
|
||||
ast_autochan_channel_unlock(internal_bridge_autochan);
|
||||
|
||||
*spyee_bridge_autochan = internal_bridge_autochan;
|
||||
|
||||
|
|
@ -639,21 +649,25 @@ static int channel_spy(struct ast_channel *chan, struct ast_autochan *spyee_auto
|
|||
struct ast_autochan *spyee_bridge_autochan = NULL;
|
||||
const char *spyer_name;
|
||||
|
||||
if (ast_check_hangup(chan) || ast_check_hangup(spyee_autochan->chan) ||
|
||||
ast_test_flag(ast_channel_flags(spyee_autochan->chan), AST_FLAG_ZOMBIE)) {
|
||||
ast_channel_lock(chan);
|
||||
if (ast_check_hangup(chan)) {
|
||||
ast_channel_unlock(chan);
|
||||
return 0;
|
||||
}
|
||||
|
||||
ast_channel_lock(chan);
|
||||
spyer_name = ast_strdupa(ast_channel_name(chan));
|
||||
ast_channel_unlock(chan);
|
||||
|
||||
ast_autochan_channel_lock(spyee_autochan);
|
||||
if (ast_check_hangup(spyee_autochan->chan)
|
||||
|| ast_test_flag(ast_channel_flags(spyee_autochan->chan), AST_FLAG_ZOMBIE)) {
|
||||
ast_autochan_channel_unlock(spyee_autochan);
|
||||
return 0;
|
||||
}
|
||||
name = ast_strdupa(ast_channel_name(spyee_autochan->chan));
|
||||
ast_autochan_channel_unlock(spyee_autochan);
|
||||
|
||||
ast_verb(2, "Spying on channel %s\n", name);
|
||||
publish_chanspy_message(chan, spyee_autochan->chan, 1);
|
||||
ast_autochan_channel_unlock(spyee_autochan);
|
||||
|
||||
memset(&csth, 0, sizeof(csth));
|
||||
ast_copy_flags(&csth.flags, flags, AST_FLAGS_ALL);
|
||||
|
|
@ -845,7 +859,7 @@ static int channel_spy(struct ast_channel *chan, struct ast_autochan *spyee_auto
|
|||
}
|
||||
|
||||
static struct ast_autochan *next_channel(struct ast_channel_iterator *iter,
|
||||
struct ast_autochan *autochan, struct ast_channel *chan)
|
||||
struct ast_channel *chan)
|
||||
{
|
||||
struct ast_channel *next;
|
||||
struct ast_autochan *autochan_store;
|
||||
|
|
@ -982,11 +996,12 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
|
|||
waitms = 100;
|
||||
num_spyed_upon = 0;
|
||||
|
||||
for (autochan = next_channel(iter, autochan, chan);
|
||||
autochan;
|
||||
prev = autochan->chan, ast_autochan_destroy(autochan),
|
||||
autochan = next_autochan ? next_autochan :
|
||||
next_channel(iter, autochan, chan), next_autochan = NULL) {
|
||||
for (autochan = next_channel(iter, chan);
|
||||
autochan;
|
||||
prev = autochan->chan,
|
||||
ast_autochan_destroy(autochan),
|
||||
autochan = next_autochan ?: next_channel(iter, chan),
|
||||
next_autochan = NULL) {
|
||||
int igrp = !mygroup;
|
||||
int ienf = !myenforced;
|
||||
|
||||
|
|
@ -1000,13 +1015,19 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
|
|||
break;
|
||||
}
|
||||
|
||||
if (ast_test_flag(flags, OPTION_BRIDGED) && !ast_channel_is_bridged(autochan->chan)) {
|
||||
ast_autochan_channel_lock(autochan);
|
||||
if (ast_test_flag(flags, OPTION_BRIDGED)
|
||||
&& !ast_channel_is_bridged(autochan->chan)) {
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (ast_check_hangup(autochan->chan) || ast_test_flag(ast_channel_flags(autochan->chan), AST_FLAG_SPYING)) {
|
||||
if (ast_check_hangup(autochan->chan)
|
||||
|| ast_test_flag(ast_channel_flags(autochan->chan), AST_FLAG_SPYING)) {
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
continue;
|
||||
}
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
|
||||
if (mygroup) {
|
||||
int num_groups = 0;
|
||||
|
|
@ -1024,11 +1045,13 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
|
|||
|
||||
/* Before dahdi scan was part of chanspy, it would use the "GROUP" variable
|
||||
* rather than "SPYGROUP", this check is done to preserve expected behavior */
|
||||
ast_autochan_channel_lock(autochan);
|
||||
if (ast_test_flag(flags, OPTION_DAHDI_SCAN)) {
|
||||
group = pbx_builtin_getvar_helper(autochan->chan, "GROUP");
|
||||
} else {
|
||||
group = pbx_builtin_getvar_helper(autochan->chan, "SPYGROUP");
|
||||
}
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
|
||||
if (!ast_strlen_zero(group)) {
|
||||
ast_copy_string(dup_group, group, sizeof(dup_group));
|
||||
|
|
@ -1056,7 +1079,9 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
|
|||
|
||||
snprintf(buffer, sizeof(buffer) - 1, ":%s:", myenforced);
|
||||
|
||||
ast_autochan_channel_lock(autochan);
|
||||
ast_copy_string(ext + 1, ast_channel_name(autochan->chan), sizeof(ext) - 1);
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
if ((end = strchr(ext, '-'))) {
|
||||
*end++ = ':';
|
||||
*end = '\0';
|
||||
|
|
@ -1078,7 +1103,9 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
|
|||
char *ptr, *s;
|
||||
|
||||
strcpy(peer_name, "spy-");
|
||||
ast_autochan_channel_lock(autochan);
|
||||
strncat(peer_name, ast_channel_name(autochan->chan), AST_NAME_STRLEN - 4 - 1);
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
if ((ptr = strchr(peer_name, '/'))) {
|
||||
*ptr++ = '\0';
|
||||
for (s = peer_name; s < ptr; s++) {
|
||||
|
|
@ -1143,12 +1170,14 @@ static int common_exec(struct ast_channel *chan, struct ast_flags *flags,
|
|||
next = ast_channel_unref(next);
|
||||
} else {
|
||||
/* stay on this channel, if it is still valid */
|
||||
ast_autochan_channel_lock(autochan);
|
||||
if (!ast_check_hangup(autochan->chan)) {
|
||||
next_autochan = ast_autochan_setup(autochan->chan);
|
||||
} else {
|
||||
/* the channel is gone */
|
||||
next_autochan = NULL;
|
||||
}
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
}
|
||||
} else if (res == 0 && ast_test_flag(flags, OPTION_EXITONHANGUP)) {
|
||||
ast_autochan_destroy(autochan);
|
||||
|
|
|
|||
|
|
@ -617,6 +617,16 @@ static void mixmonitor_save_prep(struct mixmonitor *mixmonitor, char *filename,
|
|||
}
|
||||
}
|
||||
|
||||
static int mixmonitor_autochan_is_bridged(struct ast_autochan *autochan)
|
||||
{
|
||||
int is_bridged;
|
||||
|
||||
ast_autochan_channel_lock(autochan);
|
||||
is_bridged = ast_channel_is_bridged(autochan->chan);
|
||||
ast_autochan_channel_unlock(autochan);
|
||||
return is_bridged;
|
||||
}
|
||||
|
||||
static void *mixmonitor_thread(void *obj)
|
||||
{
|
||||
struct mixmonitor *mixmonitor = obj;
|
||||
|
|
@ -674,8 +684,7 @@ static void *mixmonitor_thread(void *obj)
|
|||
ast_audiohook_unlock(&mixmonitor->audiohook);
|
||||
|
||||
if (!ast_test_flag(mixmonitor, MUXFLAG_BRIDGED)
|
||||
|| (mixmonitor->autochan->chan
|
||||
&& ast_channel_is_bridged(mixmonitor->autochan->chan))) {
|
||||
|| mixmonitor_autochan_is_bridged(mixmonitor->autochan)) {
|
||||
ast_mutex_lock(&mixmonitor->mixmonitor_ds->lock);
|
||||
|
||||
/* Write out the frame(s) */
|
||||
|
|
@ -724,11 +733,11 @@ static void *mixmonitor_thread(void *obj)
|
|||
|
||||
ast_audiohook_unlock(&mixmonitor->audiohook);
|
||||
|
||||
ast_autochan_channel_lock(mixmonitor->autochan);
|
||||
if (ast_test_flag(mixmonitor, MUXFLAG_BEEP_STOP)) {
|
||||
ast_autochan_channel_lock(mixmonitor->autochan);
|
||||
ast_stream_and_wait(mixmonitor->autochan->chan, "beep", "");
|
||||
ast_autochan_channel_unlock(mixmonitor->autochan);
|
||||
}
|
||||
ast_autochan_channel_unlock(mixmonitor->autochan);
|
||||
|
||||
ast_autochan_destroy(mixmonitor->autochan);
|
||||
|
||||
|
|
@ -800,11 +809,11 @@ static int setup_mixmonitor_ds(struct mixmonitor *mixmonitor, struct ast_channel
|
|||
return -1;
|
||||
}
|
||||
|
||||
ast_autochan_channel_lock(mixmonitor->autochan);
|
||||
if (ast_test_flag(mixmonitor, MUXFLAG_BEEP_START)) {
|
||||
ast_autochan_channel_lock(mixmonitor->autochan);
|
||||
ast_stream_and_wait(mixmonitor->autochan->chan, "beep", "");
|
||||
ast_autochan_channel_unlock(mixmonitor->autochan);
|
||||
}
|
||||
ast_autochan_channel_unlock(mixmonitor->autochan);
|
||||
|
||||
mixmonitor_ds->samp_rate = 8000;
|
||||
mixmonitor_ds->audiohook = &mixmonitor->audiohook;
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@
|
|||
struct ast_autochan {
|
||||
struct ast_channel *chan;
|
||||
AST_LIST_ENTRY(ast_autochan) list;
|
||||
ast_mutex_t lock;
|
||||
};
|
||||
|
||||
/*!
|
||||
|
|
@ -61,19 +62,24 @@ struct ast_autochan {
|
|||
* ast_autochan_channel_lock and ast_autochan_channel_unlock. An attempt to lock
|
||||
* the autochan->chan directly may result in it being changed after you've
|
||||
* retrieved the value of chan, but before you've had a chance to lock it.
|
||||
* First when chan is locked, the autochan structure is guaranteed to keep the
|
||||
* While chan is locked, the autochan structure is guaranteed to keep the
|
||||
* same channel.
|
||||
*/
|
||||
|
||||
/*!
|
||||
* \brief Lock the autochan's channel lock.
|
||||
*
|
||||
* \note We must do deadlock avoidance because the channel lock is
|
||||
* superior to the autochan lock in locking order.
|
||||
*/
|
||||
#define ast_autochan_channel_lock(autochan) \
|
||||
do { \
|
||||
struct ast_channel *autochan_chan = autochan->chan; \
|
||||
ast_channel_lock(autochan_chan); \
|
||||
if (autochan->chan == autochan_chan) { \
|
||||
break; \
|
||||
ast_mutex_lock(&(autochan)->lock); \
|
||||
while (ast_channel_trylock((autochan)->chan)) { \
|
||||
DEADLOCK_AVOIDANCE(&(autochan)->lock); \
|
||||
} \
|
||||
ast_channel_unlock(autochan_chan); \
|
||||
} while (1)
|
||||
ast_mutex_unlock(&(autochan)->lock); \
|
||||
} while (0)
|
||||
|
||||
#define ast_autochan_channel_unlock(autochan) \
|
||||
ast_channel_unlock(autochan->chan)
|
||||
|
|
|
|||
|
|
@ -46,15 +46,18 @@ struct ast_autochan *ast_autochan_setup(struct ast_channel *chan)
|
|||
if (!(autochan = ast_calloc(1, sizeof(*autochan)))) {
|
||||
return NULL;
|
||||
}
|
||||
ast_mutex_init(&autochan->lock);
|
||||
|
||||
autochan->chan = ast_channel_ref(chan);
|
||||
|
||||
ast_channel_lock(autochan->chan); /* autochan is still private, no need for ast_autochan_channel_lock() */
|
||||
ast_debug(1, "Created autochan %p to hold channel %s (%p)\n",
|
||||
autochan, ast_channel_name(chan), chan);
|
||||
|
||||
/* autochan is still private, no need for ast_autochan_channel_lock() */
|
||||
ast_channel_lock(autochan->chan);
|
||||
AST_LIST_INSERT_TAIL(ast_channel_autochans(autochan->chan), autochan, list);
|
||||
ast_channel_unlock(autochan->chan);
|
||||
|
||||
ast_debug(1, "Created autochan %p to hold channel %s (%p)\n", autochan, ast_channel_name(chan), chan);
|
||||
|
||||
return autochan;
|
||||
}
|
||||
|
||||
|
|
@ -75,6 +78,8 @@ void ast_autochan_destroy(struct ast_autochan *autochan)
|
|||
|
||||
autochan->chan = ast_channel_unref(autochan->chan);
|
||||
|
||||
ast_mutex_destroy(&autochan->lock);
|
||||
|
||||
ast_free(autochan);
|
||||
}
|
||||
|
||||
|
|
@ -84,13 +89,16 @@ void ast_autochan_new_channel(struct ast_channel *old_chan, struct ast_channel *
|
|||
|
||||
AST_LIST_APPEND_LIST(ast_channel_autochans(new_chan), ast_channel_autochans(old_chan), list);
|
||||
|
||||
/* Deadlock avoidance is not needed since the channels are already locked. */
|
||||
AST_LIST_TRAVERSE(ast_channel_autochans(new_chan), autochan, list) {
|
||||
ast_mutex_lock(&autochan->lock);
|
||||
if (autochan->chan == old_chan) {
|
||||
autochan->chan = ast_channel_unref(old_chan);
|
||||
autochan->chan = ast_channel_ref(new_chan);
|
||||
ast_channel_unref(old_chan);
|
||||
|
||||
ast_debug(1, "Autochan %p used to hold channel %s (%p) but now holds channel %s (%p)\n",
|
||||
autochan, ast_channel_name(old_chan), old_chan, ast_channel_name(new_chan), new_chan);
|
||||
}
|
||||
ast_mutex_unlock(&autochan->lock);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue