Better way to get chan and pvt lock for issue ASTERISK-17431.

Redoes -r308945 for issue ASTERISK-17431 deadlock fix for
sip_set_udptl_peer() and sip_set_rtp_peer().

* Lock the channels in the defined order and avoid the need for a deadlock
avoidance loop.

* Lock the channel before getting the pointer to the private structure to
be sure that the pointer will not change due to a masquerade or channel
hangup.

* To preserve sanity, check that chan and p->owner are the same.  (Pointer
rearangements should not happen without the protection of locks because
bad things tend to happen otherwise.)


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@326144 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Richard Mudgett
2011-07-01 21:07:22 +00:00
parent cf8b27cd39
commit 6348add664

View File

@@ -28087,25 +28087,23 @@ static struct ast_udptl *sip_get_udptl_peer(struct ast_channel *chan)
static int sip_set_udptl_peer(struct ast_channel *chan, struct ast_udptl *udptl)
{
struct sip_pvt *p;
/* Lock the channel and the private safely. */
ast_channel_lock(chan);
p = chan->tech_pvt;
if (!p) {
ast_channel_unlock(chan);
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) {
if (p->owner != chan) {
/* I suppose it could be argued that if this happens it is a bug. */
ast_debug(1, "The private is not owned by channel %s anymore.\n", chan->name);
sip_pvt_unlock(p);
ast_channel_unlock(chan);
return 0;
}
if (udptl) {
ast_udptl_get_peer(udptl, &p->udptlredirip);
} else {
@@ -28124,8 +28122,8 @@ 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);
ast_channel_unlock(chan);
return 0;
}
@@ -28232,10 +28230,21 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
struct sip_pvt *p;
int changed = 0;
/* Lock the channel and the private safely. */
ast_channel_lock(chan);
p = chan->tech_pvt;
if (!p) {
ast_channel_unlock(chan);
return -1;
}
sip_pvt_lock(p);
if (p->owner != chan) {
/* I suppose it could be argued that if this happens it is a bug. */
ast_debug(1, "The private is not owned by channel %s anymore.\n", chan->name);
sip_pvt_unlock(p);
ast_channel_unlock(chan);
return 0;
}
/* Disable early RTP bridge */
if ((instance || vinstance || tinstance) &&
@@ -28244,25 +28253,10 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
return 0;
}
/*
* 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;
}
@@ -28270,8 +28264,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;
}
@@ -28313,8 +28307,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;
}