[Asterisk-code-review] main/app.c: Memory corruption from early format destruction. (asterisk[13])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4795 )

Change subject: main/app.c: Memory corruption from early format destruction.
......................................................................


main/app.c: Memory corruption from early format destruction.

* make_silence() created a malloced silence slin frame without adding a
slin format ref.  When the frame is destroyed it will unref the slin
format that never had a ref added.  Memory corruption is expected to
follow.

* Simplified and fixed counting the number of samples in a frame list for
make_silence().

* Eliminated an unnecessary RAII_VAR associated with the make_silence()
frame.

Change-Id: I47de3f9b92635b7f8b4d72309444d6c0aee6f747
---
M main/app.c
1 file changed, 12 insertions(+), 14 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Matthew Fredrickson: Looks good to me, approved



diff --git a/main/app.c b/main/app.c
index 58aa3a0..ee7cef2 100644
--- a/main/app.c
+++ b/main/app.c
@@ -1422,22 +1422,20 @@
 	size_t size;
 	size_t datalen;
 	size_t samples = 0;
-	struct ast_frame *next;
 
 	if (!orig) {
 		return NULL;
 	}
+	do {
+		if (ast_format_cmp(orig->subclass.format, ast_format_slin) == AST_FORMAT_CMP_NOT_EQUAL) {
+			ast_log(LOG_WARNING, "Attempting to silence non-slin frame\n");
+			return NULL;
+		}
 
-	if (ast_format_cmp(orig->subclass.format, ast_format_slin) == AST_FORMAT_CMP_NOT_EQUAL) {
-		ast_log(LOG_WARNING, "Attempting to silence non-slin frame\n");
-		return NULL;
-	}
-
-	for (next = AST_LIST_NEXT(orig, frame_list);
-		 orig;
-		 orig = next, next = orig ? AST_LIST_NEXT(orig, frame_list) : NULL) {
 		samples += orig->samples;
-	}
+
+		orig = AST_LIST_NEXT(orig, frame_list);
+	} while (orig);
 
 	ast_verb(4, "Silencing %zu samples\n", samples);
 
@@ -1455,7 +1453,7 @@
 	silence->samples = samples;
 	silence->datalen = datalen;
 
-	silence->subclass.format = ast_format_slin;
+	silence->subclass.format = ao2_bump(ast_format_slin);
 
 	return silence;
 }
@@ -1661,14 +1659,13 @@
 					/* It's all good */
 					res = 0;
 				} else {
-					RAII_VAR(struct ast_frame *, silence, NULL, ast_frame_dtor);
+					struct ast_frame *silence = NULL;
 					struct ast_frame *orig = f;
 
 					if (muted) {
 						silence = make_silence(orig);
 						if (!silence) {
-							ast_log(LOG_WARNING,
-								"Error creating silence\n");
+							ast_log(LOG_WARNING, "Error creating silence\n");
 							break;
 						}
 						f = silence;
@@ -1679,6 +1676,7 @@
 						}
 						res = ast_writestream(others[x], f);
 					}
+					ast_frame_dtor(silence);
 					f = orig;
 				}
 

-- 
To view, visit https://gerrit.asterisk.org/4795
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I47de3f9b92635b7f8b4d72309444d6c0aee6f747
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>