Merged revisions 98943 via svnmerge from

https://origsvn.digium.com/svn/asterisk/branches/1.4

........
r98943 | russell | 2008-01-15 17:26:52 -0600 (Tue, 15 Jan 2008) | 25 lines

Commit a fix for some memory access errors pointed out by the valgrind2.txt
output on issue #11698.

The issue here is that it is possible for an instance of a translator to get
destroyed while the frame allocated as a part of the translator is still being
processed.  Specifically, this is possible anywhere between a call to ast_read()
and ast_frame_free(), which is _a lot_ of places in the code.  The reason this
happens is that the channel might get masqueraded during this time.  During a
masquerade, existing translation paths get destroyed.

So, this patch fixes the issue in an API and ABI compatible way.  (This one is
 for you, paravoid!)

It changes an int in ast_frame to be used as flag bits.  The 1 bit is still used
to indicate that the frame contains timing information.  Also, a second flag has
been added to indicate that the frame came from a translator.  When a frame with
this flag gets released and has this flag, a function is called in translate.c to
let it know that this frame is doing being processed.  At this point, the flag gets
cleared.  Also, if the translator was requested to be destroyed while its internal
frame still had this flag set, its destruction has been deffered until it finds out
that the frame is no longer being processed.

Admittedly, this feels like a hack.  But, it does fix the issue, and I was not able 
to think of a better solution ...

........


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@98944 65c4cc65-6c06-0410-ace0-fbb531ad65f3

4 files changed, 45 insertions, 11 deletions

diff --git a/main/abstract_jb.c b/main/abstract_jb.c
index afc6bd182..4f464796d 100644
--- a/main/abstract_jb.c
+++ b/main/abstract_jb.c
@@ -312,10 +312,10 @@ int ast_jb_put(struct ast_channel *chan, struct ast_frame *f)
 	}
 
 	/* We consider an enabled jitterbuffer should receive frames with valid timing info. */
-	if (!f->has_timing_info || f->len < 2 || f->ts < 0) {
+	if (!ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO) || f->len < 2 || f->ts < 0) {
 		ast_log(LOG_WARNING, "%s received frame with invalid timing info: "
 			"has_timing_info=%d, len=%ld, ts=%ld, src=%s\n",
-			chan->name, f->has_timing_info, f->len, f->ts, f->src);
+			chan->name, ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO), f->len, f->ts, f->src);
 		return -1;
 	}
 
diff --git a/main/frame.c b/main/frame.c
index bb386df55..9b7d15087 100644
--- a/main/frame.c
+++ b/main/frame.c
@@ -36,6 +36,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/utils.h"
 #include "asterisk/threadstorage.h"
 #include "asterisk/linkedlists.h"
+#include "asterisk/translate.h"
 
 #ifdef TRACE_FRAMES
 static int headers;
@@ -355,6 +356,9 @@ void ast_frame_free(struct ast_frame *fr, int cache)
 #endif			
 		ast_free(fr);
 	}
+
+	if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR))
+		ast_translate_frame_freed(fr);
 }
 
 /*!
@@ -366,7 +370,9 @@ struct ast_frame *ast_frisolate(struct ast_frame *fr)
 {
 	struct ast_frame *out;
 	void *newdata;
-	
+
+	ast_clear_flag(fr, AST_FRFLAG_FROM_TRANSLATOR);
+
 	if (!(fr->mallocd & AST_MALLOCD_HDR)) {
 		/* Allocate a new header if needed */
 		if (!(out = ast_frame_header_new()))
@@ -378,8 +384,8 @@ struct ast_frame *ast_frisolate(struct ast_frame *fr)
 		out->offset = fr->offset;
 		out->data = fr->data;
 		/* Copy the timing data */
-		out->has_timing_info = fr->has_timing_info;
-		if (fr->has_timing_info) {
+		ast_copy_flags(out, fr, AST_FRFLAG_HAS_TIMING_INFO);
+		if (ast_test_flag(fr, AST_FRFLAG_HAS_TIMING_INFO)) {
 			out->ts = fr->ts;
 			out->len = fr->len;
 			out->seqno = fr->seqno;
@@ -483,7 +489,7 @@ struct ast_frame *ast_frdup(const struct ast_frame *f)
 		/* Must have space since we allocated for it */
 		strcpy((char *)out->src, f->src);
 	}
-	out->has_timing_info = f->has_timing_info;
+	ast_copy_flags(out, f, AST_FRFLAG_HAS_TIMING_INFO);
 	out->ts = f->ts;
 	out->len = f->len;
 	out->seqno = f->seqno;
diff --git a/main/rtp.c b/main/rtp.c
index cdfab894b..0794fbcde 100644
--- a/main/rtp.c
+++ b/main/rtp.c
@@ -1583,7 +1583,7 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
 			ast_frame_byteswap_be(&rtp->f);
 		calc_rxstamp(&rtp->f.delivery, rtp, timestamp, mark);
 		/* Add timing data to let ast_generic_bridge() put the frame into a jitterbuf */
-		rtp->f.has_timing_info = 1;
+		ast_set_flag(&rtp->f, AST_FRFLAG_HAS_TIMING_INFO);
 		rtp->f.ts = timestamp / 8;
 		rtp->f.len = rtp->f.samples / 8;
 	} else if(rtp->f.subclass & AST_FORMAT_VIDEO_MASK) {
@@ -3028,7 +3028,7 @@ static int ast_rtp_raw_write(struct ast_rtp *rtp, struct ast_frame *f, int codec
 	if (rtp->lastts > rtp->lastdigitts)
 		rtp->lastdigitts = rtp->lastts;
 
-	if (f->has_timing_info)
+	if (ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO))
 		rtp->lastts = f->ts * 8;
 
 	/* Get a pointer to the header */
diff --git a/main/translate.c b/main/translate.c
index 2e98df92b..8bdaaf082 100644
--- a/main/translate.c
+++ b/main/translate.c
@@ -133,6 +133,18 @@ static void destroy(struct ast_trans_pvt *pvt)
 {
 	struct ast_translator *t = pvt->t;
 
+	if (ast_test_flag(&pvt->f, AST_FRFLAG_FROM_TRANSLATOR)) {
+		/* If this flag is still set, that means that the translation path has
+		 * been torn down, while we still have a frame out there being used.
+		 * When ast_frfree() gets called on that frame, this ast_trans_pvt
+		 * will get destroyed, too. */
+
+		/* Set the magic hint that this has been requested to be destroyed. */
+		pvt->datalen = -1;
+
+		return;
+	}
+
 	if (t->destroy)
 		t->destroy(pvt);
 	ast_free(pvt);
@@ -147,7 +159,7 @@ static int framein(struct ast_trans_pvt *pvt, struct ast_frame *f)
 	int samples = pvt->samples;	/* initial value */
 	
 	/* Copy the last in jb timing info to the pvt */
-	pvt->f.has_timing_info = f->has_timing_info;
+	ast_copy_flags(&pvt->f, f, AST_FRFLAG_HAS_TIMING_INFO);
 	pvt->f.ts = f->ts;
 	pvt->f.len = f->len;
 	pvt->f.seqno = f->seqno;
@@ -226,6 +238,8 @@ struct ast_frame *ast_trans_frameout(struct ast_trans_pvt *pvt,
 	f->src = pvt->t->name;
 	f->data = pvt->outbuf;
 
+	ast_set_flag(f, AST_FRFLAG_FROM_TRANSLATOR);
+
 	return f;
 }
 
@@ -304,7 +318,7 @@ struct ast_frame *ast_translate(struct ast_trans_pvt *path, struct ast_frame *f,
 	long len;
 	int seqno;
 
-	has_timing_info = f->has_timing_info;
+	has_timing_info = ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO);
 	ts = f->ts;
 	len = f->len;
 	seqno = f->seqno;
@@ -354,7 +368,7 @@ struct ast_frame *ast_translate(struct ast_trans_pvt *path, struct ast_frame *f,
 		path->nextout = ast_tvadd(path->nextout, ast_samp2tv(out->samples, format_rate(out->subclass)));
 	} else {
 		out->delivery = ast_tv(0, 0);
-		out->has_timing_info = has_timing_info;
+		ast_set2_flag(out, has_timing_info, AST_FRFLAG_HAS_TIMING_INFO);
 		if (has_timing_info) {
 			out->ts = ts;
 			out->len = len;
@@ -875,3 +889,17 @@ unsigned int ast_translate_available_formats(unsigned int dest, unsigned int src
 
 	return res;
 }
+
+void ast_translate_frame_freed(struct ast_frame *fr)
+{
+	struct ast_trans_pvt *pvt;
+
+	ast_clear_flag(fr, AST_FRFLAG_FROM_TRANSLATOR);
+
+	pvt = (struct ast_trans_pvt *) (((char *) fr) - offsetof(struct ast_trans_pvt, f));
+
+	if (pvt->datalen != -1)
+		return;
+	
+	destroy(pvt);
+}