From 161820396495a549c9a378d32136cbb5f28ef2af Mon Sep 17 00:00:00 2001 From: Joshua Colp Date: Sat, 13 May 2017 16:40:00 +0000 Subject: asterisk: Audit locking of channel when manipulating flags. When manipulating flags on a channel the channel has to be locked to guarantee that nothing else is also manipulating the flags. This change introduces locking where necessary to guarantee this. It also adds helper functions that manipulate channel flags and lock to reduce repeated code. ASTERISK-26789 Change-Id: I489280662dba0f4c50981bfc5b5a7073fef2db10 --- main/autoservice.c | 2 +- main/bridge_after.c | 2 +- main/bridge_channel.c | 2 +- main/channel.c | 38 ++++++++++++++++++++++++++++---------- main/file.c | 20 ++++++++++---------- main/manager.c | 8 ++------ main/pbx.c | 4 ++++ 7 files changed, 47 insertions(+), 29 deletions(-) (limited to 'main') diff --git a/main/autoservice.c b/main/autoservice.c index 305ab23b1..f353f0e0c 100644 --- a/main/autoservice.c +++ b/main/autoservice.c @@ -300,11 +300,11 @@ int ast_autoservice_stop(struct ast_channel *chan) res = 0; } + ast_channel_lock(chan); if (!as->orig_end_dtmf_flag) { ast_clear_flag(ast_channel_flags(chan), AST_FLAG_END_DTMF_ONLY); } - ast_channel_lock(chan); while ((f = AST_LIST_REMOVE_HEAD(&as->deferred_frames, frame_list))) { if (!((1 << f->frametype) & as->ignore_frame_types)) { ast_queue_frame_head(chan, f); diff --git a/main/bridge_after.c b/main/bridge_after.c index a41f8a548..1208b57b8 100644 --- a/main/bridge_after.c +++ b/main/bridge_after.c @@ -484,7 +484,7 @@ int ast_bridge_setup_after_goto(struct ast_channel *chan) } } else if (!ast_check_hangup(chan)) { /* Clear the outgoing flag */ - ast_clear_flag(ast_channel_flags(chan), AST_FLAG_OUTGOING); + ast_channel_clear_flag(chan, AST_FLAG_OUTGOING); if (after_bridge->specific) { goto_failed = ast_explicit_goto(chan, after_bridge->context, diff --git a/main/bridge_channel.c b/main/bridge_channel.c index b466b3c6a..eba5ae40a 100644 --- a/main/bridge_channel.c +++ b/main/bridge_channel.c @@ -2092,7 +2092,7 @@ void bridge_channel_internal_pull(struct ast_bridge_channel *bridge_channel) && (ast_channel_is_leaving_bridge(bridge_channel->chan) || bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT)) { ast_debug(2, "Channel %s will survive this bridge; clearing outgoing (dialed) flag\n", ast_channel_name(bridge_channel->chan)); - ast_clear_flag(ast_channel_flags(bridge_channel->chan), AST_FLAG_OUTGOING); + ast_channel_clear_flag(bridge_channel->chan, AST_FLAG_OUTGOING); } bridge->reconfigured = 1; diff --git a/main/channel.c b/main/channel.c index 4192c00f2..5aaeab9d2 100644 --- a/main/channel.c +++ b/main/channel.c @@ -1296,8 +1296,10 @@ int ast_channel_defer_dtmf(struct ast_channel *chan) int pre = 0; if (chan) { + ast_channel_lock(chan); pre = ast_test_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF); ast_set_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF); + ast_channel_unlock(chan); } return pre; } @@ -1305,8 +1307,9 @@ int ast_channel_defer_dtmf(struct ast_channel *chan) /*! \brief Unset defer DTMF flag on channel */ void ast_channel_undefer_dtmf(struct ast_channel *chan) { - if (chan) - ast_clear_flag(ast_channel_flags(chan), AST_FLAG_DEFER_DTMF); + if (chan) { + ast_channel_clear_flag(chan, AST_FLAG_DEFER_DTMF); + } } struct ast_channel *ast_channel_callback(ao2_callback_data_fn *cb_fn, void *arg, @@ -3518,7 +3521,7 @@ int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, in return -1; /* Only look for the end of DTMF, don't bother with the beginning and don't emulate things */ - ast_set_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_set_flag(c, AST_FLAG_END_DTMF_ONLY); /* Wait for a digit, no more than timeout_ms milliseconds total. * Or, wait indefinitely if timeout_ms is <0. @@ -3537,12 +3540,12 @@ int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, in if (errno == 0 || errno == EINTR) continue; ast_log(LOG_WARNING, "Wait failed (%s)\n", strerror(errno)); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return -1; } else if (outfd > -1) { /* The FD we were watching has something waiting */ ast_log(LOG_WARNING, "The FD we were waiting for has something waiting. Waitfordigit returning numeric 1\n"); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return 1; } else if (rchan) { int res; @@ -3556,13 +3559,13 @@ int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, in case AST_FRAME_DTMF_END: res = f->subclass.integer; ast_frfree(f); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return res; case AST_FRAME_CONTROL: switch (f->subclass.integer) { case AST_CONTROL_HANGUP: ast_frfree(f); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return -1; case AST_CONTROL_STREAM_STOP: case AST_CONTROL_STREAM_SUSPEND: @@ -3573,7 +3576,7 @@ int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, in * that perform stream control will handle this. */ res = f->subclass.integer; ast_frfree(f); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return res; case AST_CONTROL_PVT_CAUSE_CODE: case AST_CONTROL_RINGING: @@ -3608,7 +3611,7 @@ int ast_waitfordigit_full(struct ast_channel *c, int timeout_ms, int audiofd, in } } - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return 0; /* Time is up */ } @@ -5873,9 +5876,9 @@ struct ast_channel *ast_call_forward(struct ast_channel *caller, struct ast_chan } else if (caller) { /* no outgoing helper so use caller if available */ call_forward_inherit(new_chan, caller, orig); } - ast_set_flag(ast_channel_flags(new_chan), AST_FLAG_ORIGINATED); ast_channel_lock_both(orig, new_chan); + ast_channel_set_flag(new_chan, AST_FLAG_ORIGINATED); pbx_builtin_setvar_helper(new_chan, "FORWARDERNAME", forwarder); ast_party_connected_line_copy(ast_channel_connected(new_chan), ast_channel_connected(orig)); ast_party_redirecting_copy(ast_channel_redirecting(new_chan), ast_channel_redirecting(orig)); @@ -10996,3 +10999,18 @@ enum ast_channel_error ast_channel_errno(void) { return ast_channel_internal_errno(); } + +void ast_channel_set_flag(struct ast_channel *chan, unsigned int flag) +{ + ast_channel_lock(chan); + ast_set_flag(ast_channel_flags(chan), flag); + ast_channel_unlock(chan); +} + +void ast_channel_clear_flag(struct ast_channel *chan, unsigned int flag) +{ + ast_channel_lock(chan); + ast_clear_flag(ast_channel_flags(chan), flag); + ast_channel_unlock(chan); +} + diff --git a/main/file.c b/main/file.c index 11617fb7f..bb1db3ee6 100644 --- a/main/file.c +++ b/main/file.c @@ -1545,7 +1545,7 @@ static int waitstream_core(struct ast_channel *c, reverse = ""; /* Switch the channel to end DTMF frame only. waitstream_core doesn't care about the start of DTMF. */ - ast_set_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_set_flag(c, AST_FLAG_END_DTMF_ONLY); if (ast_test_flag(ast_channel_flags(c), AST_FLAG_MASQ_NOSTREAM)) orig_chan_name = ast_strdupa(ast_channel_name(c)); @@ -1577,7 +1577,7 @@ static int waitstream_core(struct ast_channel *c, res = ast_waitfor(c, ms); if (res < 0) { ast_log(LOG_WARNING, "Select failed (%s)\n", strerror(errno)); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return res; } } else { @@ -1588,11 +1588,11 @@ static int waitstream_core(struct ast_channel *c, if (errno == EINTR) continue; ast_log(LOG_WARNING, "Wait failed (%s)\n", strerror(errno)); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return -1; } else if (outfd > -1) { /* this requires cmdfd set */ /* The FD we were watching has something waiting */ - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return 1; } /* if rchan is set, it is 'c' */ @@ -1601,7 +1601,7 @@ static int waitstream_core(struct ast_channel *c, if (res > 0) { struct ast_frame *fr = ast_read(c); if (!fr) { - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return -1; } switch (fr->frametype) { @@ -1612,7 +1612,7 @@ static int waitstream_core(struct ast_channel *c, S_COR(ast_channel_caller(c)->id.number.valid, ast_channel_caller(c)->id.number.str, NULL))) { res = fr->subclass.integer; ast_frfree(fr); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return res; } } else { @@ -1628,7 +1628,7 @@ static int waitstream_core(struct ast_channel *c, "Break"); ast_frfree(fr); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return res; } } @@ -1645,7 +1645,7 @@ static int waitstream_core(struct ast_channel *c, "Break"); res = fr->subclass.integer; ast_frfree(fr); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return res; case AST_CONTROL_STREAM_REVERSE: if (!skip_ms) { @@ -1663,7 +1663,7 @@ static int waitstream_core(struct ast_channel *c, case AST_CONTROL_BUSY: case AST_CONTROL_CONGESTION: ast_frfree(fr); - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return -1; case AST_CONTROL_RINGING: case AST_CONTROL_ANSWER: @@ -1700,7 +1700,7 @@ static int waitstream_core(struct ast_channel *c, ast_sched_runq(ast_channel_sched(c)); } - ast_clear_flag(ast_channel_flags(c), AST_FLAG_END_DTMF_ONLY); + ast_channel_clear_flag(c, AST_FLAG_END_DTMF_ONLY); return (err || ast_channel_softhangup_internal_flag(c)) ? -1 : 0; } diff --git a/main/manager.c b/main/manager.c index 6f57f056e..6db52b8b4 100644 --- a/main/manager.c +++ b/main/manager.c @@ -4819,14 +4819,10 @@ static int action_redirect(struct mansession *s, const struct message *m) /* Release the bridge wait. */ if (chan1_wait) { - ast_channel_lock(chan); - ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT); - ast_channel_unlock(chan); + ast_channel_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT); } if (chan2_wait) { - ast_channel_lock(chan2); - ast_clear_flag(ast_channel_flags(chan2), AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT); - ast_channel_unlock(chan2); + ast_channel_clear_flag(chan, AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT); } chan2 = ast_channel_unref(chan2); diff --git a/main/pbx.c b/main/pbx.c index b4089abd7..cfc5f7f9f 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -4257,8 +4257,10 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c, ast_channel_pbx(c)->rtimeoutms = 10000; ast_channel_pbx(c)->dtimeoutms = 5000; + ast_channel_lock(c); autoloopflag = ast_test_flag(ast_channel_flags(c), AST_FLAG_IN_AUTOLOOP); /* save value to restore at the end */ ast_set_flag(ast_channel_flags(c), AST_FLAG_IN_AUTOLOOP); + ast_channel_unlock(c); if (ast_strlen_zero(ast_channel_exten(c))) { /* If not successful fall back to 's' - but only if there is no given exten */ @@ -4503,8 +4505,10 @@ static enum ast_pbx_result __ast_pbx_run(struct ast_channel *c, ast_pbx_hangup_handler_run(c); } + ast_channel_lock(c); ast_set2_flag(ast_channel_flags(c), autoloopflag, AST_FLAG_IN_AUTOLOOP); ast_clear_flag(ast_channel_flags(c), AST_FLAG_BRIDGE_HANGUP_RUN); /* from one round to the next, make sure this gets cleared */ + ast_channel_unlock(c); pbx_destroy(ast_channel_pbx(c)); ast_channel_pbx_set(c, NULL); -- cgit v1.2.3