frame.c: Fix off-nominal format ref leaks.

* ast_frisolate() could leak frame format refs on allocation
failures.

* Similified code in ast_frisolate() and code used by
ast_frisolate().

Change-Id: I79566d4d36b3d7801bf0c8294fcd3e9a86a2ed6d

diff --git a/main/frame.c b/main/frame.c
index b5ab9853de1c17cee85898ca452d52c45d6de82f..4261b04dd26439c6fbc50b1582a2f587898c248d 100644 (file)
--- a/main/frame.c
+++ b/main/frame.c
@@ -84,9 +84,9 @@ static struct ast_frame *ast_frame_header_new(void)
        if ((frames = ast_threadstorage_get(&frame_cache, sizeof(*frames)))) {
                if ((f = AST_LIST_REMOVE_HEAD(&frames->list, frame_list))) {
                        size_t mallocd_len = f->mallocd_hdr_len;
+
                        memset(f, 0, sizeof(*f));
                        f->mallocd_hdr_len = mallocd_len;
-                       f->mallocd = AST_MALLOCD_HDR;
                        frames->size--;
                        return f;
                }
@@ -141,12 +141,12 @@ static void __frame_free(struct ast_frame *fr, int cache)
 #endif
 
        if (fr->mallocd & AST_MALLOCD_DATA) {
-               if (fr->data.ptr)
+               if (fr->data.ptr) {
                        ast_free(fr->data.ptr - fr->offset);
+               }
        }
        if (fr->mallocd & AST_MALLOCD_SRC) {
-               if (fr->src)
-                       ast_free((void *) fr->src);
+               ast_free((void *) fr->src);
        }
        if (fr->mallocd & AST_MALLOCD_HDR) {
                if ((fr->frametype == AST_FRAME_VOICE) || (fr->frametype == AST_FRAME_VIDEO) ||
@@ -208,14 +208,14 @@ struct ast_frame *ast_frisolate(struct ast_frame *fr)
                        return NULL;
                }
                out->frametype = fr->frametype;
+               out->subclass = fr->subclass;
                if ((fr->frametype == AST_FRAME_VOICE) || (fr->frametype == AST_FRAME_VIDEO) ||
                        (fr->frametype == AST_FRAME_IMAGE)) {
-                       out->subclass.format = ao2_bump(fr->subclass.format);
-               } else {
-                       memcpy(&out->subclass, &fr->subclass, sizeof(out->subclass));
+                       ao2_bump(out->subclass.format);
                }
                out->datalen = fr->datalen;
                out->samples = fr->samples;
+               out->mallocd = AST_MALLOCD_HDR;
                out->offset = fr->offset;
                /* Copy the timing data */
                ast_copy_flags(out, fr, AST_FLAGS_ALL);
@@ -228,47 +228,64 @@ struct ast_frame *ast_frisolate(struct ast_frame *fr)
                out = fr;
        }
 
-       if (!(fr->mallocd & AST_MALLOCD_SRC) && fr->src) {
-               if (!(out->src = ast_strdup(fr->src))) {
-                       if (out != fr) {
-                               ast_free(out);
+       if (fr->src) {
+               /* The original frame has a source string */
+               if (!(fr->mallocd & AST_MALLOCD_SRC)) {
+                       /*
+                        * The original frame has a non-malloced source string.
+                        *
+                        * Duplicate the string and put it into the isolated frame
+                        * which may also be the original frame.
+                        */
+                       newdata = ast_strdup(fr->src);
+                       if (!newdata) {
+                               if (out != fr) {
+                                       ast_frame_free(out, 0);
+                               }
+                               return NULL;
                        }
-                       return NULL;
+                       out->src = newdata;
+                       out->mallocd |= AST_MALLOCD_SRC;
+               } else if (out != fr) {
+                       /* Steal the source string from the original frame. */
+                       out->src = fr->src;
+                       fr->src = NULL;
+                       fr->mallocd &= ~AST_MALLOCD_SRC;
+                       out->mallocd |= AST_MALLOCD_SRC;
                }
-       } else {
-               out->src = fr->src;
-               fr->src = NULL;
-               fr->mallocd &= ~AST_MALLOCD_SRC;
        }
 
        if (!(fr->mallocd & AST_MALLOCD_DATA))  {
+               /* The original frame has a non-malloced data buffer. */
                if (!fr->datalen) {
+                       /* Actually it's just an int so we can simply copy it. */
                        out->data.uint32 = fr->data.uint32;
-                       out->mallocd = AST_MALLOCD_HDR | AST_MALLOCD_SRC;
                        return out;
                }
-               if (!(newdata = ast_malloc(fr->datalen + AST_FRIENDLY_OFFSET))) {
-                       if (out->src != fr->src) {
-                               ast_free((void *) out->src);
-                       }
+               /*
+                * Duplicate the data buffer and put it into the isolated frame
+                * which may also be the original frame.
+                */
+               newdata = ast_malloc(fr->datalen + AST_FRIENDLY_OFFSET);
+               if (!newdata) {
                        if (out != fr) {
-                               ast_free(out);
+                               ast_frame_free(out, 0);
                        }
                        return NULL;
                }
                newdata += AST_FRIENDLY_OFFSET;
                out->offset = AST_FRIENDLY_OFFSET;
-               out->datalen = fr->datalen;
                memcpy(newdata, fr->data.ptr, fr->datalen);
                out->data.ptr = newdata;
-       } else {
+               out->mallocd |= AST_MALLOCD_DATA;
+       } else if (out != fr) {
+               /* Steal the data buffer from the original frame. */
                out->data = fr->data;
                memset(&fr->data, 0, sizeof(fr->data));
                fr->mallocd &= ~AST_MALLOCD_DATA;
+               out->mallocd |= AST_MALLOCD_DATA;
        }
 
-       out->mallocd = AST_MALLOCD_HDR | AST_MALLOCD_SRC | AST_MALLOCD_DATA;
-
        return out;
 }