[asterisk-dev] [Code Review]: Fix segfault relating to use of uninitialized aco objects / Fix misrepresentation of confbridge elements that fail to load

[Matt Jordan]

> On Aug. 27, 2012, 4:26 p.m., lathama wrote:
> > Mark, I was updating https://wiki.asterisk.org/wiki/display/AST/Modules and maybe we can better note these types of issues. I think we can standardize the load_module to rid ourselves of these problems.

So, one thing that would be good to note on the wiki page is that application/function registration should occur after module inter-dependency checks and configuration parsing.  It is much less likely for an ast_register_* method to return failure than it is for a configuration load to fail.

That would have prevented part (2) of this bug.


- Matt


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


On Aug. 27, 2012, 4:11 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2086/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2012, 4:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In ASTERISK-20305, a crash was reported in app_page that stemmed from the use of an uninitialized aco_type pointer by app_confbridge. The reason that the pointer was uninitialized was that app_confbridge had failed to load due to a missing confbridge.conf file. Even though app_confbridge had not loaded, the CONFBRIDGE dialplan function was still accessible from app_page.
> 
> This patch fixes the problem in two ways:
> 
> 1) The actual crash is fixed by mending config_options.c not to assume that aco_type->internal and aco_info->internal are allocated/initialized. Since it is possible to call aco functions with invalid data, we need to be prepared for it.
> 
> 2) app_confbridge's dialplan functions, dialplan applications, and manager actions do not need to stay registered if the module declines to load. The simple fix for this was to place the one condition under which the module could decline loading at the top of the load_module() function.
> 
> It should be noted that app_confbridge is almost certainly not the only app that registers applications, funtions, and manager actions and fails to unregister them when the module declines loading. I will be opening up an issue report soon to get this taken care of across the board.
> 
> 
> This addresses bug ASTERISK-20305.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20305
> 
> 
> Diffs
> -----
> 
>   /branches/11/apps/app_confbridge.c 371779 
>   /branches/11/main/config_options.c 371779 
> 
> Diff: https://reviewboard.asterisk.org/r/2086/diff
> 
> 
> Testing
> -------
> 
> I ran the reporter of ASTERISK-20305's test:
> 1) Start Asterisk with no confbridge.conf file
> 2) Run app_page.
> 
> Prior to this set of fixes, this would crash.
> 
> I also ran another test the reporter mentioned:
> 1) Start Asterisk with no confbridge.conf file
> 2) Create a confbridge.conf file
> 3) run "module load app_confbridge.so" from the Asterisk CLI
> 
> Prior to this set of fixes, the "module load" would fail because the dialplan function "CONFBRIDGE" was already registered. With this set of changes, the module loads as expected now.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120827/4ae486cb/attachment.htm>