From d72a2966da39c56c7dbf14e04ba7a6aa302aa2f1 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Thu, 5 Apr 2018 17:40:52 -0500 Subject: chan_sip.c: Fix INVITE with replaces channel ref leak. Given the below call scenario: A -> Ast1 -> B C <- Ast2 <- B 1) A calls B through Ast1 2) B calls C through Ast2 3) B transfers A to C When party B transfers A to C, B sends a REFER to Ast1 causing Ast1 to send an INVITE with replaces to Ast2. Ast2 then leaks a channel ref of the channel between Ast1 and Ast2. Channel ref leaks are easily seen in the CLI "core show channels" output. The leaked channels appear in the output but you can do nothing with them and they never go away unless you restart Asterisk. * Properly account for the channel refs when imparting a channel into a bridge when handling an INVITE with replaces in handle_invite_replaces(). The ast_bridge_impart() function steals a channel ref but the code didn't account for how many refs were held by the code at the time and which ref was stolen. * Eliminated RAII_VAR in handle_invite_replaces(). ASTERISK-27740 Change-Id: I7edbed774314b55acf0067b2762bfe984ecaa9a4 --- channels/chan_sip.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 138021e82..c357785ab 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -25856,7 +25856,7 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, int *nounlock, struct sip_pvt *replaces_pvt, struct ast_channel *replaces_chan) { struct ast_channel *c; - RAII_VAR(struct ast_bridge *, bridge, NULL, ao2_cleanup); + struct ast_bridge *bridge; if (req->ignore) { return 0; @@ -25872,6 +25872,7 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, } append_history(p, "Xfer", "INVITE/Replace received"); + /* Get a ref to ensure the channel cannot go away on us. */ c = ast_channel_ref(p->owner); /* Fake call progress */ @@ -25891,16 +25892,22 @@ static int handle_invite_replaces(struct sip_pvt *p, struct sip_request *req, ast_channel_unlock(replaces_chan); if (bridge) { + /* + * We have two refs of the channel. One is held in c and the other + * is notionally represented by p->owner. The impart is "stealing" + * the p->owner ref on success so the bridging system can have + * control of when the channel is hung up. + */ if (ast_bridge_impart(bridge, c, replaces_chan, NULL, AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) { ast_hangup(c); - ast_channel_unref(c); } + ao2_ref(bridge, -1); } else { ast_channel_move(replaces_chan, c); ast_hangup(c); - ast_channel_unref(c); } + ast_channel_unref(c); sip_pvt_lock(p); return 0; } -- cgit v1.2.3