From 5be186d7c5003a7a90a0db83df6f0ecc59d6c77e Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Thu, 12 Sep 2013 16:44:34 +0000 Subject: [PATCH] core_local: Fix memory corruption race condition. The masquerade super test is failing on v12 with high fence violations and crashing. The fence violations are showing that party id allocated memory strings are somehow getting corrupted in the bridge_reconfigured_connected_line_update() function. The invalid string values happen to be the freed memory fill pattern. After much puzzling, I deduced that the bridge_reconfigured_connected_line_update() is copying a string out of the source channel's caller party id struct just as another thread is updating it with a new value. The copying thread is using the old string pointer being freed by the updating thread. A search of the code found the unreal_colp_redirect_indicate() routine updating the caller party id's without holding the channel lock. A latent bug in v1.8 and v11 hatched in v12 because of the bridging and connected line changes. :) (issue ASTERISK-22221) Reported by: Matt Jordan Review: https://reviewboard.asterisk.org/r/2839/ ........ Merged revisions 398938 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@398939 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- main/core_unreal.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/main/core_unreal.c b/main/core_unreal.c index 99ddc51632..07748b4428 100644 --- a/main/core_unreal.c +++ b/main/core_unreal.c @@ -441,10 +441,18 @@ static int unreal_queue_indicate(struct ast_unreal_pvt *p, struct ast_channel *a */ static int unreal_colp_redirect_indicate(struct ast_unreal_pvt *p, struct ast_channel *ast, int condition) { + struct ast_channel *my_chan; + struct ast_channel *my_owner; struct ast_channel *this_channel; struct ast_channel *the_other_channel; int isoutbound; int res = 0; + unsigned char frame_data[1024]; + struct ast_frame f = { + .frametype = AST_FRAME_CONTROL, + .subclass.integer = condition, + .data.ptr = frame_data, + }; /* * A connected line update frame may only contain a partial @@ -458,7 +466,8 @@ static int unreal_colp_redirect_indicate(struct ast_unreal_pvt *p, struct ast_ch * redirecting information, which is why it is handled here as * well. */ - ao2_lock(p); + ast_channel_unlock(ast); + ast_unreal_lock_all(p, &my_chan, &my_owner); isoutbound = AST_UNREAL_IS_OUTBOUND(ast, p); if (isoutbound) { this_channel = p->chan; @@ -468,13 +477,6 @@ static int unreal_colp_redirect_indicate(struct ast_unreal_pvt *p, struct ast_ch the_other_channel = p->chan; } if (the_other_channel) { - unsigned char frame_data[1024]; - struct ast_frame f = { - .frametype = AST_FRAME_CONTROL, - .subclass.integer = condition, - .data.ptr = frame_data, - }; - if (condition == AST_CONTROL_CONNECTED_LINE) { ast_connected_line_copy_to_caller(ast_channel_caller(the_other_channel), ast_channel_connected(this_channel)); @@ -484,9 +486,20 @@ static int unreal_colp_redirect_indicate(struct ast_unreal_pvt *p, struct ast_ch f.datalen = ast_redirecting_build_data(frame_data, sizeof(frame_data), ast_channel_redirecting(this_channel), NULL); } - res = unreal_queue_frame(p, isoutbound, &f, ast, 1); + } + if (my_chan) { + ast_channel_unlock(my_chan); + ast_channel_unref(my_chan); + } + if (my_owner) { + ast_channel_unlock(my_owner); + ast_channel_unref(my_owner); + } + if (the_other_channel) { + res = unreal_queue_frame(p, isoutbound, &f, ast, 0); } ao2_unlock(p); + ast_channel_lock(ast); return res; }