From 9c70c883697d63e6f16b0b06b688a6131423c7e1 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Fri, 11 Aug 2017 16:31:45 -0500 Subject: channel: Fix topology API locking. * ast_channel_request_stream_topology_change() must not be called with any channel locks held. * ast_channel_stream_topology_changed() must be called with only the passed channel lock held. ASTERISK-27212 Change-Id: I843de7956d9f1cc7cc02025aea3463d8fe19c691 --- include/asterisk/channel.h | 4 ++-- main/channel.c | 9 ++++++++- tests/test_stream.c | 4 ++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index f0fe5b212..3dfbe61d9 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -4934,7 +4934,7 @@ int ast_channel_is_multistream(struct ast_channel *chan); * \param topology The new stream topology * \param change_source The source that initiated the change * - * \pre chan is locked + * \note Absolutely _NO_ channel locks should be held before calling this function. * * \retval 0 request has been accepted to be attempted * \retval -1 request could not be attempted @@ -4956,7 +4956,7 @@ int ast_channel_request_stream_topology_change(struct ast_channel *chan, * \param chan The channel to provide notice to * \param topology The new stream topology * - * \pre chan is locked + * \pre chan is locked Absolutely _NO_ other channels can be locked. * * \retval 0 success * \retval -1 failure diff --git a/main/channel.c b/main/channel.c index 632d47247..74de9caac 100644 --- a/main/channel.c +++ b/main/channel.c @@ -10843,22 +10843,29 @@ enum ast_channel_error ast_channel_errno(void) int ast_channel_request_stream_topology_change(struct ast_channel *chan, struct ast_stream_topology *topology, void *change_source) { + int res; + ast_assert(chan != NULL); ast_assert(topology != NULL); + ast_channel_lock(chan); if (!ast_channel_is_multistream(chan) || !ast_channel_tech(chan)->indicate) { + ast_channel_unlock(chan); return -1; } if (ast_stream_topology_equal(ast_channel_get_stream_topology(chan), topology)) { ast_debug(3, "Topology of %s already matches what is requested so ignoring topology change request\n", ast_channel_name(chan)); + ast_channel_unlock(chan); return 0; } ast_channel_internal_set_stream_topology_change_source(chan, change_source); - return ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology)); + res = ast_channel_tech(chan)->indicate(chan, AST_CONTROL_STREAM_TOPOLOGY_REQUEST_CHANGE, topology, sizeof(topology)); + ast_channel_unlock(chan); + return res; } int ast_channel_stream_topology_changed(struct ast_channel *chan, struct ast_stream_topology *topology) diff --git a/tests/test_stream.c b/tests/test_stream.c index 28983e05c..8c887048c 100644 --- a/tests/test_stream.c +++ b/tests/test_stream.c @@ -1768,7 +1768,9 @@ AST_TEST_DEFINE(stream_topology_change_request_from_application_non_multistream) ast_test_validate_cleanup(test, change_res == -1, res, done); ast_test_validate_cleanup(test, !pvt->indicated_change_request, res, done); + ast_channel_lock(mock_channel); change_res = ast_channel_stream_topology_changed(mock_channel, topology); + ast_channel_unlock(mock_channel); ast_test_validate_cleanup(test, change_res == -1, res, done); ast_test_validate_cleanup(test, !pvt->indicated_changed, res, done); @@ -1876,7 +1878,9 @@ AST_TEST_DEFINE(stream_topology_change_request_from_application) ast_test_validate_cleanup(test, !change_res, res, done); ast_test_validate_cleanup(test, pvt->indicated_change_request, res, done); + ast_channel_lock(mock_channel); change_res = ast_channel_stream_topology_changed(mock_channel, topology); + ast_channel_unlock(mock_channel); ast_test_validate_cleanup(test, !change_res, res, done); ast_test_validate_cleanup(test, pvt->indicated_changed, res, done); -- cgit v1.2.3