[asterisk-dev] [Code Review] 2617: Prevent crash during synchronous AMI origination by ref bumping returned channel

[Matt Jordan]

> On June 12, 2013, 4:49 p.m., Joshua Colp wrote:
> > /trunk/include/asterisk/pbx.h, lines 1103-1105
> > <https://reviewboard.asterisk.org/r/2617/diff/1/?file=39559#file39559line1103>
> >
> >     Any reason you kept the whole locking thing in play, and not just returned with ref bumped?

In manager's case, you want to access properties of the channel. While that shouldn't change out from underneath you in normal circumstances, it could if the channel was immediately masqueraded.

So, it is safer to keep it locked.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2617/#review8878
-----------------------------------------------------------


On June 12, 2013, 4:45 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2617/
> -----------------------------------------------------------
> 
> (Updated June 12, 2013, 4:45 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> *NOTE*: this only happens on trunk.
> 
> The originate APIs allow callers to provide a pointer to a channel that will point to the originated channel if the function call succeeds. This is used by AMI to provide channel information when the originate is performed synchronously. Unfortunately, if the originate fails in certain ways, the outbound channel is already disposed of during the dialing itself. This results in the channel being improperly dereferenced by the internal originate function in pbx.c.
> 
> This patch ref bumps the channel to prevent this from occurring. Callers must now unlock and unref the channel (which is more in line with general channel management guidelines anyway).
> 
> This only affects manager, as it is the only consumer of this API function that actually passes in a channel pointer.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/pbx.c 391524 
>   /trunk/main/manager.c 391524 
>   /trunk/include/asterisk/pbx.h 391524 
> 
> Diff: https://reviewboard.asterisk.org/r/2617/diff/
> 
> 
> Testing
> -------
> 
> One of the Dial tests in the Asterisk Test Suite caught this. With this change, it stopped crashing and reported the failure of the originate correctly.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130613/6096ea26/attachment.htm>