[asterisk-dev] [Code Review]: Fix dangerous setting of channel application data when doing built-in attended transfer or AMI Bridge action

[Matt Jordan]

> On Sept. 11, 2012, 9:39 a.m., Matt Jordan wrote:
> > /branches/1.8/main/features.c, lines 959-961
> > <https://reviewboard.asterisk.org/r/2103/diff/2/?file=31167#file31167line959>
> >
> >     Some WARNING log messages (or ERROR) when the memory allocation fails would be good.
> 
> Sean Bright wrote:
>     This is already taken care of by the internal memory management code I believe.

Whoops.  I knew that it was if MALLOC_DEBUG was defined, but didn't think that was the case otherwise.  I missed the MALLOC_FAILURE_MSG in utils.h - my bad.  Disregard!


- Matt


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


On Sept. 10, 2012, 11:58 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2103/
> -----------------------------------------------------------
> 
> (Updated Sept. 10, 2012, 11:58 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> While attempting to track down the crash reported in ASTERISK-20335, I was able to trigger a series of valgrind warnings due to dangerous setting of channel's data pointer in ast_bridge_call_thread().
> 
> The data on channel A would be set to point to the name of channel B and vice-versa. The issue here is that if channel B gets freed, then channel A's data points to freed memory.
> 
> My method for fixing this is to essentially copy what Kevin Fleming recently did in main/cel.c where there was another channel data-related issue. I have created a datastore that contains a copy of the data string, and pointed the channel's data pointer at this allocated string. When the channel is destroyed, the datastore is destroyed along with it and the allocated string is freed. This way the lifetime of the channel's data is tied to its own lifetime rather than to the lifetime of a different channel.
> 
> 
> This addresses bug ASTERISK-20335.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20335
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/features.c 371589 
> 
> Diff: https://reviewboard.asterisk.org/r/2103/diff
> 
> 
> Testing
> -------
> 
> Ran a test scenario that involved an attended transfer failure. Without the patch, valgrind reports many invalid reads once the second channel hangs up. With the patch, valgrind reports no errors.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120911/70e0111c/attachment.htm>