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/trunk@114208 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/include/asterisk/dsp.h b/include/asterisk/dsp.h
index 81735d625023022369012e5a733d4e4f2d7d6f19..871a51ac9a2e37acbbfd58ff7ef9627b88e84823 100644 (file)
--- a/include/asterisk/dsp.h
+++ b/include/asterisk/dsp.h
@@ -138,4 +138,16 @@ int ast_dsp_reload(void);
 
 int ast_dsp_init(void);
 
+/*!
+ * \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 ad4b8292006bfd3aed303f939ceba2b1b0444f2d..719048b02c371242bdcc52268256785a549b0fbc 100644 (file)
--- 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 f2d47df595349cc4885cf82d1cae03b53d6a829f..935bcb0bf713529efce073749d1bf24f1a10d298 100644 (file)
--- a/main/dsp.c
+++ b/main/dsp.c
@@ -388,6 +388,7 @@ struct ast_dsp {
        digit_detect_state_t digit_state;
        tone_detect_state_t cng_tone_state;
        tone_detect_state_t ced_tone_state;
+       int destroy;
 };
 
 static void mute_fragment(struct ast_dsp *dsp, fragment_t *fragment)
@@ -1310,6 +1311,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)) {
@@ -1319,6 +1321,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;
        }
 
@@ -1424,6 +1427,7 @@ done:
                if (chan) 
                        ast_queue_frame(chan, af);
                ast_frfree(af);
+               ast_set_flag(outf, AST_FRFLAG_FROM_DSP);
                return outf;
        } else {
                return af;
@@ -1474,6 +1478,16 @@ 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. */
+
+               dsp->destroy = 1;
+
+               return;
+       }
        ast_free(dsp);
 }
 
@@ -1632,3 +1646,16 @@ int ast_dsp_reload(void)
        return _dsp_init(1);
 }
 
+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->destroy)
+               return;
+       
+       ast_dsp_free(dsp);
+}

diff --git a/main/frame.c b/main/frame.c
index 3971393fd2780d26f1c79cab9e22e41e9146307f..c49e8e661933f5b5953c3195252dff22af3e9c50 100644 (file)
--- 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;
@@ -307,6 +308,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;
@@ -356,6 +359,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 */