From d73fed119d7739a5f54b5963cac872fae03222d4 Mon Sep 17 00:00:00 2001
From: Richard Mudgett <rmudgett@digium.com>
Date: Wed, 11 Jan 2017 14:59:24 -0600
Subject: 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
---
 main/frame.c | 69 +++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 26 deletions(-)

(limited to 'main')

diff --git a/main/frame.c b/main/frame.c
index 92b92b6e2..833408089 100644
--- 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;
 }
 
-- 
cgit v1.2.3