From 1dd0e2df7c60e87675feef911f10b17cbb33e731 Mon Sep 17 00:00:00 2001 From: Mark Michelson Date: Thu, 17 Apr 2008 16:48:47 +0000 Subject: [PATCH] Merged revisions 114208 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r114208 | mmichelson | 2008-04-17 11:40:12 -0500 (Thu, 17 Apr 2008) | 20 lines Merged revisions 114207 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r114207 | mmichelson | 2008-04-17 11:28:03 -0500 (Thu, 17 Apr 2008) | 12 lines 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 ........ ................ git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.6.0@114210 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- include/asterisk/dsp.h | 12 ++++++++++++ include/asterisk/frame.h | 4 ++++ main/dsp.c | 31 +++++++++++++++++++++++++++++++ main/frame.c | 4 ++++ 4 files changed, 51 insertions(+) diff --git a/include/asterisk/dsp.h b/include/asterisk/dsp.h index 10ed53d1b1..6f635a83d8 100644 --- a/include/asterisk/dsp.h +++ b/include/asterisk/dsp.h @@ -115,4 +115,16 @@ int ast_dsp_get_tstate(struct ast_dsp *dsp); /*! \brief Get tcount (Threshold counter) */ int ast_dsp_get_tcount(struct ast_dsp *dsp); +/*! + * \brief Hint that a frame from a dsp was freed + * + * This is called from ast_frame_free if AST_FRFLAG_FROM_DSP is set. This occurs + * because it is possible for the dsp to be freed while someone still holds a reference + * to the frame that is in that dsp. This has been known to happen when the dsp on a Zap + * channel detects a busy signal. The channel is hung up, and the application that read the + * frame to begin with still has a reference to the frame. + * + * \return nothing + */ +void ast_dsp_frame_freed(struct ast_frame *fr); #endif /* _ASTERISK_DSP_H */ diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h index ad4b829200..719048b02c 100644 --- a/include/asterisk/frame.h +++ b/include/asterisk/frame.h @@ -130,6 +130,10 @@ enum { * 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 diff --git a/main/dsp.c b/main/dsp.c index 6945464d47..f24030ebbd 100644 --- a/main/dsp.c +++ b/main/dsp.c @@ -1241,6 +1241,7 @@ struct ast_frame *ast_dsp_process(struct ast_channel *chan, struct ast_dsp *dsp, 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)) { @@ -1250,6 +1251,7 @@ struct ast_frame *ast_dsp_process(struct ast_channel *chan, struct ast_dsp *dsp, dsp->f.subclass = AST_CONTROL_BUSY; ast_frfree(af); ast_debug(1, "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) || (dsp->features & DSP_FEATURE_FAX_DETECT))) { @@ -1271,6 +1273,7 @@ struct ast_frame *ast_dsp_process(struct ast_channel *chan, struct ast_dsp *dsp, if (chan) ast_queue_frame(chan, af); ast_frfree(af); + ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP); return &dsp->f; } } else { @@ -1297,6 +1300,7 @@ struct ast_frame *ast_dsp_process(struct ast_channel *chan, struct ast_dsp *dsp, 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)); @@ -1314,6 +1318,7 @@ struct ast_frame *ast_dsp_process(struct ast_channel *chan, struct ast_dsp *dsp, if (chan) ast_queue_frame(chan, af); ast_frfree(af); + ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP); return &dsp->f; } } @@ -1331,6 +1336,7 @@ struct ast_frame *ast_dsp_process(struct ast_channel *chan, struct ast_dsp *dsp, if (chan) ast_queue_frame(chan, af); ast_frfree(af); + ast_set_flag(&dsp->f, AST_FRFLAG_FROM_DSP); return &dsp->f; } } @@ -1403,6 +1409,17 @@ void ast_dsp_set_features(struct ast_dsp *dsp, int features) 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 flag to destroy the dsp when the frame is freed */ + dsp->freqcount = -1; + + return; + } ast_free(dsp); } @@ -1516,3 +1533,17 @@ int ast_dsp_get_tcount(struct ast_dsp *dsp) { 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); +} diff --git a/main/frame.c b/main/frame.c index 07ce936f2c..dc471eebe6 100644 --- a/main/frame.c +++ b/main/frame.c @@ -37,6 +37,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #include "asterisk/threadstorage.h" #include "asterisk/linkedlists.h" #include "asterisk/translate.h" +#include "asterisk/dsp.h" #ifdef TRACE_FRAMES static int headers; @@ -308,6 +309,8 @@ void ast_frame_free(struct ast_frame *fr, int cache) { 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; @@ -357,6 +360,7 @@ struct ast_frame *ast_frisolate(struct ast_frame *fr) 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 */