From 73254a28cb5317827e938d7fe07fe3adaa3fcc69 Mon Sep 17 00:00:00 2001 From: Alec L Davis Date: Fri, 25 Feb 2011 18:52:53 +0000 Subject: [PATCH] Fix Deadlock with attended transfer of SIP call 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 | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 75df29961f..73f24ad43f 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -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; } -- 2.47.3