Skip to content

Commit

Permalink
bridge_native_rtp.c: Fix reentrancy framehook crash.
Browse files Browse the repository at this point in the history
If two channels enter different native rtp bridges at the same time it is
possible that the framehook interface data pointer can be corrupted
because the struct variable was declared static.

* Fixed the reentrancy corruption by changing the framehook interface
struct static variable to a stack local variable.

* Moved the hook.data assignment outside of the channel lock.  It did not
need the lock's protection.  It probably was giving a false sense of
security.

The testsuite
channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/bob_hangs_up
test caught this with MALLOC_DEBUG and DO_CRASH enabled.

Change-Id: If9e35b97d19209b0f984941c1d8eb5f7c55eea91
  • Loading branch information
rmudgett9125 committed Dec 28, 2017
1 parent 23aa20b commit fa36f9c
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions bridges/bridge_native_rtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ static int native_rtp_bridge_compatible(struct ast_bridge *bridge)
static int native_rtp_bridge_framehook_attach(struct ast_bridge_channel *bridge_channel)
{
struct native_rtp_bridge_channel_data *data = bridge_channel->tech_pvt;
static struct ast_framehook_interface hook = {
struct ast_framehook_interface hook = {
.version = AST_FRAMEHOOK_INTERFACE_VERSION,
.event_cb = native_rtp_framehook,
.destroy_cb = __ao2_cleanup,
Expand All @@ -771,9 +771,10 @@ static int native_rtp_bridge_framehook_attach(struct ast_bridge_channel *bridge_
ast_debug(2, "Bridge '%s'. Attaching hook data %p to '%s'\n",
bridge_channel->bridge->uniqueid, data, ast_channel_name(bridge_channel->chan));

ast_channel_lock(bridge_channel->chan);
/* We're giving 1 ref to the framehook and keeping the one from the alloc for ourselves */
hook.data = ao2_bump(data->hook_data);

ast_channel_lock(bridge_channel->chan);
data->hook_data->id = ast_framehook_attach(bridge_channel->chan, &hook);
ast_channel_unlock(bridge_channel->chan);
if (data->hook_data->id < 0) {
Expand Down

0 comments on commit fa36f9c

Please sign in to comment.