[asterisk-commits] mmichelson: branch 1.4 r114207 - in /branches/1.4: include/asterisk/ main/

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Thu Apr 17 11:28:03 2008
New Revision: 114207

URL: http://svn.digium.com/view/asterisk?view=rev&rev=114207
Log:
It was possible for a reference to a frame which was part of a freed DSP to still be
referenced, leading to memory corruption and eventual crashes. This code change ensures
that the dsp is freed when we are finished with the frame. This change is very similar
to a change Russell made with translators back a month or so ago.

(closes issue #11999)
Reported by: destiny6628
Patches:
      11999.patch uploaded by putnopvut (license 60)
Tested by: destiny6628, victoryure


Modified:
    branches/1.4/include/asterisk/frame.h
    branches/1.4/main/dsp.c
    branches/1.4/main/frame.c

Modified: branches/1.4/include/asterisk/frame.h
URL: http://svn.digium.com/view/asterisk/branches/1.4/include/asterisk/frame.h?view=diff&rev=114207&r1=114206&r2=114207
==============================================================================
--- branches/1.4/include/asterisk/frame.h (original)
+++ branches/1.4/include/asterisk/frame.h Thu Apr 17 11:28:03 2008
@@ -131,6 +131,10 @@
 	 *  The translator can not be free'd if the frame inside of it still has
 	 *  this flag set. */
 	AST_FRFLAG_FROM_TRANSLATOR = (1 << 1),
+	/*! This frame came from a dsp and is still the original frame.
+	 *  The dsp cannot be free'd if the frame inside of it still has
+	 *  this flag set. */
+	AST_FRFLAG_FROM_DSP = (1 << 2),
 };
 
 /*! \brief Data structure associated with a single frame of data

Modified: branches/1.4/main/dsp.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/dsp.c?view=diff&rev=114207&r1=114206&r2=114207
==============================================================================
--- branches/1.4/main/dsp.c (original)
+++ branches/1.4/main/dsp.c Thu Apr 17 11:28:03 2008
@@ -1486,6 +1486,7 @@
 		memset(&dsp->f, 0, sizeof(dsp->f));
 		dsp->f.frametype = AST_FRAME_NULL;
 		ast_frfree(af);
+		ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 		return &dsp->f;
 	}
 	if ((dsp->features & DSP_FEATURE_BUSY_DETECT) && ast_dsp_busydetect(dsp)) {
@@ -1494,7 +1495,7 @@
 		dsp->f.frametype = AST_FRAME_CONTROL;
 		dsp->f.subclass = AST_CONTROL_BUSY;
 		ast_frfree(af);
-		ast_log(LOG_DEBUG, "Requesting Hangup because the busy tone was detected on channel %s\n", chan->name);
+		ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 		return &dsp->f;
 	}
 	if ((dsp->features & DSP_FEATURE_DTMF_DETECT)) {
@@ -1516,6 +1517,7 @@
 					if (chan)
 						ast_queue_frame(chan, af);
 					ast_frfree(af);
+					ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 					return &dsp->f;
 				}
 			} else {
@@ -1542,6 +1544,7 @@
 							ast_queue_frame(chan, af);
 						ast_frfree(af);
 					}
+					ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 					return &dsp->f;
 				} else {
 					memset(&dsp->f, 0, sizeof(dsp->f));
@@ -1559,6 +1562,7 @@
 					if (chan)
 						ast_queue_frame(chan, af);
 					ast_frfree(af);
+					ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 					return &dsp->f;
 				}
 			}
@@ -1575,6 +1579,7 @@
 					if (chan)
 						ast_queue_frame(chan, af);
 					ast_frfree(af);
+					ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 					return &dsp->f;
 				}
 			} else {
@@ -1588,6 +1593,7 @@
 					if (chan)
 						ast_queue_frame(chan, af);
 					ast_frfree(af);
+					ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP);
 					return &dsp->f;
 				}
 			}
@@ -1658,6 +1664,17 @@
 
 void ast_dsp_free(struct ast_dsp *dsp)
 {
+	if (ast_test_flag(&dsp->f, AST_FRFLAG_FROM_DSP)) {
+		/* If this flag is still set, that means that the dsp's destruction 
+		 * 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. */
+		dsp->freqcount = -1;
+
+		return;
+	}
 	free(dsp);
 }
 
@@ -1786,3 +1803,17 @@
 {
 	return dsp->tcount;
 }
+
+void ast_dsp_frame_freed(struct ast_frame *fr)
+{
+	struct ast_dsp *dsp;
+
+	ast_clear_flag(fr, AST_FRFLAG_FROM_DSP);
+
+	dsp = (struct ast_dsp *) (((char *) fr) - offsetof(struct ast_dsp, f));
+
+	if (dsp->freqcount != -1)
+		return;
+	
+	ast_dsp_free(dsp);
+}

Modified: branches/1.4/main/frame.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/main/frame.c?view=diff&rev=114207&r1=114206&r2=114207
==============================================================================
--- branches/1.4/main/frame.c (original)
+++ branches/1.4/main/frame.c Thu Apr 17 11:28:03 2008
@@ -321,6 +321,8 @@
 {
 	if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR))
 		ast_translate_frame_freed(fr);
+	else if (ast_test_flag(fr, AST_FRFLAG_FROM_DSP))
+		ast_dsp_frame_freed(fr);
 
 	if (!fr->mallocd)
 		return;
@@ -370,6 +372,7 @@
 	void *newdata;
 
 	ast_clear_flag(fr, AST_FRFLAG_FROM_TRANSLATOR);
+	ast_clear_flag(fr, AST_FRFLAG_FROM_DSP);
 
 	if (!(fr->mallocd & AST_MALLOCD_HDR)) {
 		/* Allocate a new header if needed */