diff options
author | Kinsey Moore <kmoore@digium.com> | 2014-03-25 16:06:57 +0000 |
---|---|---|
committer | Kinsey Moore <kmoore@digium.com> | 2014-03-25 16:06:57 +0000 |
commit | a4890eddfb4f98029c046f490e03e2ae9170dee7 (patch) | |
tree | 6a4761189dddf9b1257d427e4845edd86616a5e8 /channels/chan_sip.c | |
parent | eb0a982f8c485b9a205f3bcf775ded7837a0fae4 (diff) |
chan_sip: Fix incorrect use of timers
If update_provisional_keepalive() is called while
send_provisional_keepalive_full() is waiting on the PVT lock, then
pvt->provisional_keepalive_sched_id will be changed to a new sched_id
value by update_provisional_keepalive(), but that new sched_id then may
be overwritten with -1 by send_provisional_keepalive_full(), killing
the pvt's reference to a schedule and "leaking" the reference.
(closes issue ASTERISK-22079)
Review: https://reviewboard.asterisk.org/r/3368/
Reported by: Jamuel Starkey, Matteo, Leif Madsen, Steve Davies
Patches:
provisional_keepalive_fix.diff uploaded by Steve Davies (license 5012)
........
Merged revisions 411088 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 411089 from http://svn.asterisk.org/svn/asterisk/branches/11
........
Merged revisions 411091 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@411092 65c4cc65-6c06-0410-ace0-fbb531ad65f3
Diffstat (limited to 'channels/chan_sip.c')
-rw-r--r-- | channels/chan_sip.c | 23 |
1 files changed, 12 insertions, 11 deletions
diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 23c502c82..dbd3903a3 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -4712,8 +4712,20 @@ static int send_provisional_keepalive_full(struct sip_pvt *pvt, int with_sdp) const char *msg = NULL; struct ast_channel *chan; int res = 0; + int old_sched_id = pvt->provisional_keepalive_sched_id; chan = sip_pvt_lock_full(pvt); + /* Check that nothing has changed while we were waiting for the lock */ + if (old_sched_id != pvt->provisional_keepalive_sched_id) { + /* Keepalive has been cancelled or rescheduled, clean up and leave */ + if (chan) { + ast_channel_unlock(chan); + chan = ast_channel_unref(chan); + } + sip_pvt_unlock(pvt); + dialog_unref(pvt, "dialog ref for provisional keepalive"); + return 0; + } if (!pvt->last_provisional || !strncasecmp(pvt->last_provisional, "100", 3)) { msg = "183 Session Progress"; @@ -4739,20 +4751,9 @@ static int send_provisional_keepalive_full(struct sip_pvt *pvt, int with_sdp) sip_pvt_unlock(pvt); -#if 0 - /* - * XXX BUG TODO - * - * Without this code, it appears as if this function is leaking its - * reference to the sip_pvt. However, adding it introduces a crash. - * This points to some sort of reference count imbalance elsewhere, - * but I'm not sure where ... - */ if (!res) { dialog_unref(pvt, "dialog ref for provisional keepalive"); } -#endif - return res; } |