[asterisk-dev] [Code Review] 3055: pbx.c: add lock around ast_exten use to prevent memory corruption

[Scott Griepentrog]

> On Dec. 6, 2013, 4:14 p.m., Mark Michelson wrote:
> > /branches/1.8/main/pbx.c, lines 7804-7805
> > <https://reviewboard.asterisk.org/r/3055/diff/2/?file=49178#file49178line7804>
> >
> >     I don't understand what this is supposed to be doing. This lock doesn't actually protect anything. I suspect this may narrow the window of the race condition but it seems like whatever dangerous operation was occurring can still occur since that dangerous operation is not surrounded by the lock.

The lock is being used to delay the release of the objects - once that begins the lock is immediately released to prevent slowdown of dialplan execution.


> On Dec. 6, 2013, 4:14 p.m., Mark Michelson wrote:
> > /branches/1.8/main/pbx.c, lines 825-827
> > <https://reviewboard.asterisk.org/r/3055/diff/2/?file=49178#file49178line825>
> >
> >     Since these functions are local to pbx.c, declare them static. Also use "static" on the function definitions.
> >     
> >     Since the new lock is a mutex, why do you have read and write lock functions?

I mistakenly duplicated the code behind the contexts lock (which was read/write but was behind the scenes changed to mutex to resolve issue) instead of creating it as a read/write lock.


- Scott


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


On Dec. 6, 2013, 4:44 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3055/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2013, 4:44 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Bugs: AST-1179 and AST-1246
>     https://issues.asterisk.org/jira/browse/AST-1179
>     https://issues.asterisk.org/jira/browse/AST-1246
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> During dialplan execution in pbx_extension_helper(), the contexts global read lock is used prevent changes to the dialplan.  This patch adds a lock between that point and until after the extension previously found is no longer being referenced -- after variable substitution, but before app execution.  This new lock prevents the extension's allocation from being released by module reload when it is still being used in an expression evaluation.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/pbx.c 403430 
> 
> Diff: https://reviewboard.asterisk.org/r/3055/diff/
> 
> 
> Testing
> -------
> 
> Testsuite test 'dialplan_stress' (https://reviewboard.asterisk.org/r/3056/) created to isolate this problem and test this fix.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131206/2cea15ca/attachment.html>