diff options
author | mmichelson <mmichelson@f38db490-d61c-443f-a65b-d21fe96a405b> | 2008-11-20 18:06:48 +0000 |
---|---|---|
committer | mmichelson <mmichelson@f38db490-d61c-443f-a65b-d21fe96a405b> | 2008-11-20 18:06:48 +0000 |
commit | b2d12f85251b611e5c223c8a8dd5a2c33e79c461 (patch) | |
tree | 5ab33adcd3b8ea1bc888882660bcbf6a487588b7 | |
parent | 93c58b4bdbdfa703e9fb4137b7dd01ce6df00075 (diff) |
There was an issue when attempting to reference an embedded
frame in a freed ast_filestream. This patch makes use of the
ao2 functions to make sure that we do not free an ast_filestream
structure until the embedded ast_frame has been "freed" as well.
(closes issue #13496)
Reported by: fst-onge
Patches:
filestream_frame_1_4.diff uploaded by putnopvut (license 60)
Tested by: putnopvut
Closes AST-89
git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.4@158126 f38db490-d61c-443f-a65b-d21fe96a405b
-rw-r--r-- | include/asterisk/file.h | 15 | ||||
-rw-r--r-- | include/asterisk/frame.h | 4 | ||||
-rw-r--r-- | main/file.c | 115 | ||||
-rw-r--r-- | main/frame.c | 8 |
4 files changed, 101 insertions, 41 deletions
diff --git a/include/asterisk/file.h b/include/asterisk/file.h index 636309bc4..52b0a9f18 100644 --- a/include/asterisk/file.h +++ b/include/asterisk/file.h @@ -402,6 +402,21 @@ off_t ast_tellstream(struct ast_filestream *fs); */ struct ast_frame *ast_readframe(struct ast_filestream *s); +/*!\brief destroy a filestream using an ast_frame as input + * + * This is a hack that is used also by the ast_trans_pvt and + * ast_dsp structures. When a structure contains an ast_frame + * pointer as one of its fields. It may be that the frame is + * still used after the outer structure is freed. This leads to + * invalid memory accesses. This function allows for us to hold + * off on destroying the ast_filestream until we are done using + * the ast_frame pointer that is part of it + * + * \param fr The ast_frame that is part of an ast_filestream we wish + * to free. + */ +void ast_filestream_frame_freed(struct ast_frame *fr); + /*! Initialize file stuff */ /*! * Initializes all the various file stuff. Basically just registers the cli stuff diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h index 30686efde..6ee131eb2 100644 --- a/include/asterisk/frame.h +++ b/include/asterisk/frame.h @@ -135,6 +135,10 @@ enum { * The dsp cannot be free'd if the frame inside of it still has * this flag set. */ AST_FRFLAG_FROM_DSP = (1 << 2), + /*! This frame came from a filestream and is still the original frame. + * The filestream cannot be free'd if the frame inside of it still has + * this flag set. */ + AST_FRFLAG_FROM_FILESTREAM = (1 << 3), }; /*! \brief Data structure associated with a single frame of data diff --git a/main/file.c b/main/file.c index d891000f7..cfa5bf330 100644 --- a/main/file.c +++ b/main/file.c @@ -52,6 +52,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #include "asterisk/pbx.h" #include "asterisk/linkedlists.h" #include "asterisk/module.h" +#include "asterisk/astobj2.h" /* * The following variable controls the layout of localized sound files. @@ -296,12 +297,57 @@ static int exts_compare(const char *exts, const char *type) return 0; } +static void filestream_destructor(void *arg) +{ + char *cmd = NULL; + size_t size = 0; + struct ast_filestream *f = arg; + + /* Stop a running stream if there is one */ + if (f->owner) { + if (f->fmt->format < AST_FORMAT_MAX_AUDIO) { + f->owner->stream = NULL; + AST_SCHED_DEL(f->owner->sched, f->owner->streamid); +#ifdef HAVE_DAHDI + ast_settimeout(f->owner, 0, NULL, NULL); +#endif + } else { + f->owner->vstream = NULL; + AST_SCHED_DEL(f->owner->sched, f->owner->vstreamid); + } + } + /* destroy the translator on exit */ + if (f->trans) + ast_translator_free_path(f->trans); + + if (f->realfilename && f->filename) { + size = strlen(f->filename) + strlen(f->realfilename) + 15; + cmd = alloca(size); + memset(cmd,0,size); + snprintf(cmd,size,"/bin/mv -f %s %s",f->filename,f->realfilename); + ast_safe_system(cmd); + } + + if (f->filename) + free(f->filename); + if (f->realfilename) + free(f->realfilename); + if (f->fmt->close) + f->fmt->close(f); + fclose(f->f); + if (f->vfs) + ast_closestream(f->vfs); + if (f->orig_chan_name) + free((void *) f->orig_chan_name); + ast_module_unref(f->fmt->module); +} + static struct ast_filestream *get_filestream(struct ast_format *fmt, FILE *bfile) { struct ast_filestream *s; int l = sizeof(*s) + fmt->buf_size + fmt->desc_size; /* total allocation size */ - if ( (s = ast_calloc(1, l)) == NULL) + if ( (s = ao2_alloc(l, filestream_destructor)) == NULL) return NULL; s->fmt = fmt; s->f = bfile; @@ -657,6 +703,10 @@ struct ast_frame *ast_readframe(struct ast_filestream *s) int whennext = 0; if (s && s->fmt) f = s->fmt->read(s, &whennext); + if (f) { + ast_set_flag(f, AST_FRFLAG_FROM_FILESTREAM); + ao2_ref(s, +1); + } return f; } @@ -813,46 +863,22 @@ int ast_stream_rewind(struct ast_filestream *fs, off_t ms) int ast_closestream(struct ast_filestream *f) { - char *cmd = NULL; - size_t size = 0; - /* Stop a running stream if there is one */ - if (f->owner) { - if (f->fmt->format < AST_FORMAT_MAX_AUDIO) { - f->owner->stream = NULL; - AST_SCHED_DEL(f->owner->sched, f->owner->streamid); -#ifdef HAVE_DAHDI - ast_settimeout(f->owner, 0, NULL, NULL); -#endif - } else { - f->owner->vstream = NULL; - AST_SCHED_DEL(f->owner->sched, f->owner->vstreamid); - } - } - /* destroy the translator on exit */ - if (f->trans) - ast_translator_free_path(f->trans); - if (f->realfilename && f->filename) { - size = strlen(f->filename) + strlen(f->realfilename) + 15; - cmd = alloca(size); - memset(cmd,0,size); - snprintf(cmd,size,"/bin/mv -f %s %s",f->filename,f->realfilename); - ast_safe_system(cmd); + if (ast_test_flag(&f->fr, AST_FRFLAG_FROM_FILESTREAM)) { + /* If this flag is still set, it essentially means that the reference + * count of f is non-zero. We can't destroy this filestream until + * whatever is using the filestream's frame has finished. + * + * Since this was called, however, we need to remove the reference from + * when this filestream was first allocated. That way, when the embedded + * frame is freed, the refcount will reach 0 and we can finish destroying + * this filestream properly. + */ + ao2_ref(f, -1); + return 0; } - - if (f->filename) - free(f->filename); - if (f->realfilename) - free(f->realfilename); - if (f->fmt->close) - f->fmt->close(f); - fclose(f->f); - if (f->vfs) - ast_closestream(f->vfs); - if (f->orig_chan_name) - free((void *) f->orig_chan_name); - ast_module_unref(f->fmt->module); - free(f); + + ao2_ref(f, -1); return 0; } @@ -1261,6 +1287,17 @@ int ast_waitstream_exten(struct ast_channel *c, const char *context) -1, -1, context); } +void ast_filestream_frame_freed(struct ast_frame *fr) +{ + struct ast_filestream *fs; + + ast_clear_flag(fr, AST_FRFLAG_FROM_FILESTREAM); + + fs = (struct ast_filestream *) (((char *) fr) - offsetof(struct ast_filestream, fr)); + + ao2_ref(fs, -1); +} + /* * if the file name is non-empty, try to play it. * Return 0 if success, -1 if error, digit if interrupted by a digit. diff --git a/main/frame.c b/main/frame.c index d64fa09a7..6768d0a98 100644 --- a/main/frame.c +++ b/main/frame.c @@ -45,6 +45,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$") #include "asterisk/linkedlists.h" #include "asterisk/translate.h" #include "asterisk/dsp.h" +#include "asterisk/file.h" #ifdef TRACE_FRAMES static int headers; @@ -320,10 +321,13 @@ static void frame_cache_cleanup(void *data) void ast_frame_free(struct ast_frame *fr, int cache) { - if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR)) + if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR)) { ast_translate_frame_freed(fr); - else if (ast_test_flag(fr, AST_FRFLAG_FROM_DSP)) + } else if (ast_test_flag(fr, AST_FRFLAG_FROM_DSP)) { ast_dsp_frame_freed(fr); + } else if (ast_test_flag(fr, AST_FRFLAG_FROM_FILESTREAM)) { + ast_filestream_frame_freed(fr); + } if (!fr->mallocd) return; |