Protect ast_filestream object when on a channel

The ast_filestream object gets tacked on to a channel via
chan->timingdata.  It's a reference counted object, but the reference
count isn't used when putting it on a channel.  It's theoretically
possible for another thread to interfere with the channel while it's
unlocked and cause the filestream to get destroyed.

Use the astobj2 reference count to make sure that as long as this code
path is holding on the ast_filestream and passing it into the file.c
playback code, that it knows it's valid.

Bug reported by Leif Madsen.

Review: https://reviewboard.asterisk.org/r/3135/

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

diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h
index a3f0c3d292f79106c5a215c353255ff89ab08ba9..d232e5c6afce4ac7ef12a148a687c2600f0de2e7 100644 (file)
--- a/include/asterisk/channel.h
+++ b/include/asterisk/channel.h
@@ -951,6 +951,10 @@ enum {
         * to continue.
         */
        AST_FLAG_BRIDGE_DUAL_REDIRECT_WAIT = (1 << 22),
+       /*!
+        * The data on chan->timingdata is an astobj2 object.
+        */
+       AST_FLAG_TIMINGDATA_IS_AO2_OBJ = (1 << 23),
 };
 
 /*! \brief ast_bridge_config flags */
@@ -2260,6 +2264,7 @@ int ast_autoservice_ignore(struct ast_channel *chan, enum ast_frame_type ftype);
  * \version 1.6.1 changed samples parameter to rate, accomodates new timing methods
  */
 int ast_settimeout(struct ast_channel *c, unsigned int rate, int (*func)(const void *data), void *data);
+int ast_settimeout_full(struct ast_channel *c, unsigned int rate, int (*func)(const void *data), void *data, unsigned int is_ao2_obj);
 
 /*!
  * \brief Transfer a channel (if supported).

diff --git a/main/channel.c b/main/channel.c
index 28f0741efe0417834748cbd7d16c38fc229306ea..b923649e429006e77b0e24dc34e5772fb64130f4 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -3544,6 +3544,11 @@ int ast_waitfordigit(struct ast_channel *c, int ms)
 }
 
 int ast_settimeout(struct ast_channel *c, unsigned int rate, int (*func)(const void *data), void *data)
+{
+       return ast_settimeout_full(c, rate, func, data, 0);
+}
+
+int ast_settimeout_full(struct ast_channel *c, unsigned int rate, int (*func)(const void *data), void *data, unsigned int is_ao2_obj)
 {
        int res;
        unsigned int real_rate = rate, max_rate;
@@ -3568,9 +3573,20 @@ int ast_settimeout(struct ast_channel *c, unsigned int rate, int (*func)(const v
 
        res = ast_timer_set_rate(c->timer, real_rate);
 
+       if (c->timingdata && ast_test_flag(c, AST_FLAG_TIMINGDATA_IS_AO2_OBJ)) {
+               ao2_ref(c->timingdata, -1);
+       }
+
        c->timingfunc = func;
        c->timingdata = data;
 
+       if (data && is_ao2_obj) {
+               ao2_ref(data, 1);
+               ast_set_flag(c, AST_FLAG_TIMINGDATA_IS_AO2_OBJ);
+       } else {
+               ast_clear_flag(c, AST_FLAG_TIMINGDATA_IS_AO2_OBJ);
+       }
+
        if (func == NULL && rate == 0 && c->fdno == AST_TIMING_FD) {
                /* Clearing the timing func and setting the rate to 0
                 * means that we don't want to be reading from the timingfd
@@ -3877,9 +3893,17 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
                                /* save a copy of func/data before unlocking the channel */
                                int (*func)(const void *) = chan->timingfunc;
                                void *data = chan->timingdata;
+                               int got_ref = 0;
+                               if (data && ast_test_flag(chan, AST_FLAG_TIMINGDATA_IS_AO2_OBJ)) {
+                                       ao2_ref(data, 1);
+                                       got_ref = 1;
+                               }
                                chan->fdno = -1;
                                ast_channel_unlock(chan);
                                func(data);
+                               if (got_ref) {
+                                       ao2_ref(data, -1);
+                               }
                        } else {
                                ast_timer_set_rate(chan->timer, 0);
                                chan->fdno = -1;

diff --git a/main/file.c b/main/file.c
index abcdfd65d561e8a877df9939f0b4718bfd24e3f2..b19382e579172a336dc5e789c71fd6b8a3519a3f 100644 (file)
--- a/main/file.c
+++ b/main/file.c
@@ -782,7 +782,7 @@ static enum fsread_res ast_readaudio_callback(struct ast_filestream *s)
 
                        rate = (unsigned int) roundf(samp_rate / ((float) whennext));
 
-                       ast_settimeout(s->owner, rate, ast_fsread_audio, s);
+                       ast_settimeout_full(s->owner, rate, ast_fsread_audio, s, 1);
                } else {
                        s->owner->streamid = ast_sched_add(s->owner->sched, 
                                whennext / (ast_format_rate(s->fmt->format) / 1000), ast_fsread_audio, s);