channel: ast_write frame wrongly freed after call to audiohooks
authorKevin Harwell <kharwell@digium.com>
Mon, 5 Jun 2017 16:27:32 +0000 (11:27 -0500)
committerKevin Harwell <kharwell@digium.com>
Mon, 5 Jun 2017 16:27:32 +0000 (11:27 -0500)
ASTERISK-26419 introduced a bug when calling ast_audiohook_write_list in
ast_write. It would free the frame given to ast_write if the frame returned
by ast_audiohook_write_list was different than the given one. The frame give
to ast_write should never be freed within that function. It is the caller's
resposibility to free the frame after writing (or when it its done with it).
By freeing it within ast_write this of course led to some memory corruption
problems.

This patch makes it so the frame given to ast_write is no longer freed within
the function. The frame returned by ast_audiohook_write_list is now subsequently
used in ast_write and is freed later. It is freed either after translate if the
frame returned by translate is different, or near the end of ast_write prior to
function exit.

ASTERISK-26973 #close

Change-Id: Ic9085ba5f555eeed12f6e565a638c3649695988b

main/channel.c

index 4451e52..58f960a 100644 (file)
@@ -5040,26 +5040,21 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
                        apply_plc(chan, fr);
                }
 
+               f = fr;
+
                /*
                 * Send frame to audiohooks if present, if frametype is linear (else, later as per
                 * previous behavior)
                 */
                if ((stream == default_stream) && ast_channel_audiohooks(chan)) {
                        if (ast_format_cache_is_slinear(fr->subclass.format)) {
-                               struct ast_frame *old_frame;
                                hooked = 1;
-                               old_frame = fr;
-                               fr = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_WRITE, fr);
-                               if (old_frame != fr) {
-                                       ast_frfree(old_frame);
-                               }
+                               f = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_WRITE, fr);
                        }
                }
 
                /* If the frame is in the raw write format, then it's easy... just use the frame - otherwise we will have to translate */
-               if ((stream != default_stream) || ast_format_cmp(fr->subclass.format, ast_channel_rawwriteformat(chan)) == AST_FORMAT_CMP_EQUAL) {
-                       f = fr;
-               } else {
+               if ((stream == default_stream) && ast_format_cmp(fr->subclass.format, ast_channel_rawwriteformat(chan)) != AST_FORMAT_CMP_EQUAL) {
                        if (ast_format_cmp(ast_channel_writeformat(chan), fr->subclass.format) != AST_FORMAT_CMP_EQUAL) {
                                struct ast_str *codec_buf = ast_str_alloca(AST_FORMAT_CAP_NAMES_LEN);
 
@@ -5085,7 +5080,18 @@ int ast_write_stream(struct ast_channel *chan, int stream_num, struct ast_frame
                                        break;
                                }
                        }
-                       f = ast_channel_writetrans(chan) ? ast_translate(ast_channel_writetrans(chan), fr, 0) : fr;
+
+                       if (ast_channel_writetrans(chan)) {
+                               struct ast_frame *trans_frame = ast_translate(ast_channel_writetrans(chan), f, 0);
+                               if (trans_frame != f && f != fr) {
+                                       /*
+                                        * If translate gives us a new frame and so did the audio
+                                        * hook then we need to free the one from the audio hook.
+                                        */
+                                       ast_frfree(f);
+                               }
+                               f = trans_frame;
+                       }
                }
 
                if (!f) {