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/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/12@398938 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/main/core_unreal.c b/main/core_unreal.c
index 99ddc51632d8d5abed9a9cda6b88795c11eb9945..07748b4428be8da5d0bc15ccea9bbd424135ceaa 100644 (file)
--- 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;
 }