[Home]

Summary:ASTERISK-25506: [patch]CONFBRIDGE failure after an app_confbrige.so module reload results in segfault or error/warning messages.
Reporter:Frederic LE FOLL (flefoll)Labels:
Date Opened:2015-10-29 07:55:30Date Closed:2017-05-05 10:35:41
Priority:MinorRegression?
Status:Closed/CompleteComponents:Applications/app_confbridge
Versions:13.6.0 Frequency of
Occurrence
Related
Issues:
Environment:CentOS 6Attachments:( 0) ASTERISK-25506-conf_config_parser.patch
Description:h2. Problem description:

Set(CONFBRIDGE(menu,template)=...) fails after an app_confbrige.so module reload.
Option 1: segmentation fault.
Option 2: ERROR and WARNING.

The messages when no segmentation fault:
 "ERROR ... aco_process_var: Error parsing template=xxx at line 0 of CONFBRIDGE"
 "WARNING ...  func_confbridge_helper: CONFBRIDGE(menu,template) cannot be set to '...'. Invalid type, option, or value."

h2. How to reproduce:

extensions.conf:
{noformat}
 [test]
 exten=1,1,Set(CONFBRIDGE(menu,template)=xxx) ; where xxx is a menu defined in confbridge.conf
{noformat}

CLI:
 > console dial 1@test
 [... nothing special here ...]
 > module reload app_confbridge.so
 Module 'app_confbridge.so' reloaded successfully.
     -- Reloading module 'app_confbridge.so' (Conference Bridge Application)
 CLI> console dial 1@test
 ERROR ... aco_process_var: Error parsing template=xxx at line 0 of CONFBRIDGE
 WARNING ... func_confbridge_helper: CONFBRIDGE(menu,template) cannot be set to 'xxx'. Invalid type, option, or value.

h2. Analysis:

Upon "module reload app_confbridge.so", apps/confbridge/conf_config_parser.c:conf_reload_config() is called.
Long story made short, nice cooperation between conf_config_parser.c, main/config_options.c and  main/config.c:
# aco_process_config() will allocate memory for configuration reload (allocation through confbridge_cfg_alloc()) and store the pointer into confbridge structure field cfg_handle.internal->pending.
# ast_config_load2(), because config file has not changed, will free this "pending" memory (through ast_config_destroy()) and return a "pseudo"-pointer CONFIG_STATUS_FILEUNCHANGED).
# However, CONFBRIDGE(menu,template), processed by menu_template_handler(), will read (through aco_pending_config()) and try to use this "pending" pointer that has not been reset and now points to freed memory. Contents not guaranteed !

Who uses this "pending" configuration memory in confbridge ? It seems that only menu_template_handler() and verify_default_profiles() use it.
verify_default_profiles() aims at verifying a new configuration. The use of aco_pending_config() seems logical.
menu_template_handler() just needs to consult the valid config, just as handle_cli_confgbridge_show_...() functions, for instance.

h2. Proposal :

In apps/confbridge/conf_config_parser.c:menu_template_handler(),
replace: struct confbridge_cfg *cfg = aco_pending_config(&cfg_info);
with: RAII_VAR(struct confbridge_cfg *, cfg, ao2_global_obj_ref(cfg_handle), ao2_cleanup);
This allows menu_template_handler() to read configuration just like handle_cli_confgbridge_show_...() functions do, and this fixes the problem : no more core dump, no more "Invalid type, option, or value".
Comments:By: Asterisk Team (asteriskteam) 2015-10-29 07:55:31.830-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].
By: Rusty Newton (rnewton) 2015-11-04 14:16:59.327-0600

[~flefoll] Please sign the submission agreement and attach your proposed patch to the issue. If you have additional time please then walk it through the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].  Thanks!
By: Frederic LE FOLL (flefoll) 2015-11-05 06:51:21.119-0600

Battling right now with git. "The remote end hung up unexpectedly", thus here is a patch using the "Not Preferred" method...

It seems you still have my old License Agreement (Disclaimer) from 2006 - same username, same email address - because when I try to fill in a new License Agreement, I obtain "You already have a license that is approved or pending review".
By: Rusty Newton (rnewton) 2015-11-11 07:50:43.221-0600

[~flefoll] if you have issues with Gerrit be sure to ask questions on the asterisk-dev list or the #asterisk-dev IRC channel.

http://www.asterisk.org/community/discuss

Regarding filling out a new license agreement.. I'm not sure how to do that. I'll ask legal and let you know.
By: Rusty Newton (rnewton) 2015-11-11 15:23:40.916-0600

bq. Regarding filling out a new license agreement.. I'm not sure how to do that. I'll ask legal and let you know.

Err I misread your comment. I see that you were just stating that you already have a license on file. Yes, you do! :)
By: Friendly Automation (friendly-automation) 2017-05-05 10:35:42.236-0500

Change 5579 merged by Jenkins2:
app_confbridge:  Fix reference to cfg in menu_template_handler

[https://gerrit.asterisk.org/5579|https://gerrit.asterisk.org/5579]
By: Friendly Automation (friendly-automation) 2017-05-05 13:28:19.698-0500

Change 5581 merged by George Joseph:
app_confbridge:  Fix reference to cfg in menu_template_handler

[https://gerrit.asterisk.org/5581|https://gerrit.asterisk.org/5581]
By: Friendly Automation (friendly-automation) 2017-05-05 13:28:26.122-0500

Change 5580 merged by George Joseph:
app_confbridge:  Fix reference to cfg in menu_template_handler

[https://gerrit.asterisk.org/5580|https://gerrit.asterisk.org/5580]