core_local: Fix memory corruption race condition.
authorRichard Mudgett <rmudgett@digium.com>
Thu, 12 Sep 2013 16:44:34 +0000 (16:44 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Thu, 12 Sep 2013 16:44:34 +0000 (16:44 +0000)
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

index 99ddc51..07748b4 100644 (file)
@@ -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;
 }