[Asterisk-code-review] audiohook.c: Fix freeing a frame and still using it. (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/7231 )

Change subject: audiohook.c: Fix freeing a frame and still using it.
......................................................................

audiohook.c: Fix freeing a frame and still using it.

Memory corruption happened to the media frame caches when an audio hook
freed a frame when it shouldn't.  I think the freed frame was because a
jitter buffer interpolated a missing frame and the audio hook
unconditionally freed it.

* Made audiohook.c:audio_audiohook_write_list() not free an interpolated
frame if it is the same frame as what was passed into the routine.

* Made plc.c:normalise_history() use memmove() instead of memcpy() on a
memory block that could overlap.  Found by valgrind investigating this
issue.

ASTERISK-27238
ASTERISK-27412

Change-Id: I548d86894281fc4529aefeb9f161f2131ecc6fde
---
M main/audiohook.c
M main/plc.c
2 files changed, 4 insertions(+), 2 deletions(-)

Approvals:
  Corey Farrell: Looks good to me, but someone else must approve
  Sean Bright: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Approved for Submit



diff --git a/main/audiohook.c b/main/audiohook.c
index 869cb75..cb3c4bc 100644
--- a/main/audiohook.c
+++ b/main/audiohook.c
@@ -947,7 +947,9 @@
 	 * rely on actual media being present to do things.
 	 */
 	if (!middle_frame->data.ptr) {
-		ast_frfree(middle_frame);
+		if (middle_frame != start_frame) {
+			ast_frfree(middle_frame);
+		}
 		return start_frame;
 	}
 
diff --git a/main/plc.c b/main/plc.c
index 1f02069..4421e7a 100644
--- a/main/plc.c
+++ b/main/plc.c
@@ -98,7 +98,7 @@
 	if (s->buf_ptr == 0)
 		return;
 	memcpy(tmp, s->history, sizeof(int16_t)*s->buf_ptr);
-	memcpy(s->history, s->history + s->buf_ptr, sizeof(int16_t) * (PLC_HISTORY_LEN - s->buf_ptr));
+	memmove(s->history, s->history + s->buf_ptr, sizeof(int16_t) * (PLC_HISTORY_LEN - s->buf_ptr));
 	memcpy(s->history + PLC_HISTORY_LEN - s->buf_ptr, tmp, sizeof(int16_t) * s->buf_ptr);
 	s->buf_ptr = 0;
 }

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I548d86894281fc4529aefeb9f161f2131ecc6fde
Gerrit-Change-Number: 7231
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171116/73427b66/attachment.html>