]> git.ipfire.org Git - thirdparty/asterisk.git/commitdiff
Fix Deadlock with attended transfer of SIP call
authorAlec L Davis <sivad.a@paradise.net.nz>
Fri, 25 Feb 2011 18:52:53 +0000 (18:52 +0000)
committerAlec L Davis <sivad.a@paradise.net.nz>
Fri, 25 Feb 2011 18:52:53 +0000 (18:52 +0000)
Call path
  sip_set_rtp_peer (locks chan then pvt)
   transmit_reinvite_with_sdp
    try_suggested_sip_codec
     pbx_builtin_getvar_helper (locks p->owner)

But by the time p->owner lock was attempted, seems as though chan and p->owner were different.

So in sip_set_rtp_peer, lock pvt first then lock p->owner using deadlocking methods.

(closes issue #18837)
Reported by: alecdavis
Patches:
      bug18837-trunk.diff3.txt uploaded by alecdavis (license 585)
Tested by: alecdavis, Irontec, ZX81, cmaj

Review: [https://reviewboard.asterisk.org/r/1126/]

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

channels/chan_sip.c

index 75df29961f0e14cc940043ed0ca81b665276785b..73f24ad43f9aeba0ec3e6398ed6f05485a01b7a5 100644 (file)
@@ -27615,7 +27615,20 @@ static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
        if (!p) {
                return -1;
        }
+       /*
+        * Lock both the pvt and it's owner safely.
+        */
        sip_pvt_lock(p);
+       while (p->owner && ast_channel_trylock(p->owner)) {
+               sip_pvt_unlock(p);
+               usleep(1);
+               sip_pvt_lock(p);
+       }
+
+       if (!p->owner) {
+               sip_pvt_unlock(p);
+               return 0;
+       }
        if (udptl) {
                ast_udptl_get_peer(udptl, &p->udptlredirip);
        } else {
@@ -27634,6 +27647,7 @@ static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
        }
        /* Reset lastrtprx timer */
        p->lastrtprx = p->lastrtptx = time(NULL);
+       ast_channel_unlock(p->owner);
        sip_pvt_unlock(p);
        return 0;
 }
@@ -27750,12 +27764,25 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
        if (!ast_bridged_channel(chan) && !sip_cfg.directrtpsetup)      /* We are in early state */
                return 0;
 
-       ast_channel_lock(chan);
+       /*
+        * Lock both the pvt and it's owner safely.
+        */
        sip_pvt_lock(p);
+       while (p->owner && ast_channel_trylock(p->owner)) {
+               sip_pvt_unlock(p);
+               usleep(1);
+               sip_pvt_lock(p);
+       }
+
+       if (!p->owner) {
+               sip_pvt_unlock(p);
+               return 0;
+       }
+
        if (p->alreadygone) {
                /* If we're destroyed, don't bother */
+               ast_channel_unlock(p->owner);
                sip_pvt_unlock(p);
-               ast_channel_unlock(chan);
                return 0;
        }
 
@@ -27763,8 +27790,8 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
           that are known to be behind a NAT, then stop the process now
        */
        if (nat_active && !ast_test_flag(&p->flags[0], SIP_DIRECT_MEDIA_NAT)) {
+               ast_channel_unlock(p->owner);
                sip_pvt_unlock(p);
-               ast_channel_unlock(chan);
                return 0;
        }
 
@@ -27806,8 +27833,8 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
        }
        /* Reset lastrtprx timer */
        p->lastrtprx = p->lastrtptx = time(NULL);
+       ast_channel_unlock(p->owner);
        sip_pvt_unlock(p);
-       ast_channel_unlock(chan);
        return 0;
 }