[asterisk-dev] [Code Review] 2839: Fix memory corruption race condition.

[Matt Jordan]

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



/branches/12/main/core_unreal.c
<https://reviewboard.asterisk.org/r/2839/#comment18793>

    We need to bump the reference count on ast here prior to unlocking it, particularly since we queue a frame on it later in this method after it has been unlocked/locked.
    
    In particular, since my_chan is ast, we could theoretically nuke the channel ourselves if, while we have it locked, something else decrements the reference count to 1 and we deref it to 0 on line 492.


- Matt Jordan


On Sept. 10, 2013, 12:42 a.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2839/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2013, 12:42 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22221
>     https://issues.asterisk.org/jira/browse/ASTERISK-22221
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The masquerade super test is failing on v12 with high fence violations and crashing.  The fence violations are showing that party id allocated memory strings are somehow getting corrupted in the bridge_reconfigured_connected_line_update() function.  The invalid string values happen to be the freed memory fill pattern.
> 
> After much puzzling, I deduced that the bridge_reconfigured_connected_line_update() is copying a string out of the source channel's caller party id struct just as another thread is updating it with a new value.  The copying thread is using the old string pointer being freed by the updating thread.  A search of the code found the unreal_colp_redirect_indicate() routine updating the caller party id's without holding the channel lock.
> 
> A latent bug in v1.8 and v11 hatched in v12 because of the bridging and connected line changes. :)
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/core_unreal.c 398732 
> 
> Diff: https://reviewboard.asterisk.org/r/2839/diff/
> 
> 
> Testing
> -------
> 
> Its hard to prove a negative.
> I manually did a local chain of 300 several times and MALLOC_DEBUG did not complain of any high fence violations.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130910/ba70573b/attachment-0001.htm>