[Asterisk-code-review] app confbridge: Fix reference to cfg in menu template handler (asterisk[14])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/5580 )

Change subject: app_confbridge:  Fix reference to cfg in menu_template_handler
......................................................................


app_confbridge:  Fix reference to cfg in menu_template_handler

menu_template_handler wasn't properly accounting for the fact that
it might be called both during a load/reload (which isn't really
valid but not prevented) and by a dialplan function.  In both cases
it was attempting to use the "pending" config which wasn't valid in
the latter case.  aco_process_config is also partly to blame because
it wasn't properly cleaning "pending" up when a reload was done and
no changes were made.  Both of these contributed to a crash if
CONFBRIDGE(menu,template) was called in a dialplan after a reload.

* aco_process_config now sets info->internal->pending to NULL
  after it unrefs it although this isn't strictly necessary in the
  context of this fix.
* menu_template_handler now uses the "current" config and silently
  ignores any attempt to be called as a result of someone uses the
  "template" parameter in the conf file.

Luckily there's no other place in the codebase where
aco_pending_config is used outside of aco_process_config.

ASTERISK-25506 #close
Reported-by: Frederic LE FOLL

Change-Id: Ib349a17d3d088f092480b19addd7122fcaac21a7
---
M apps/confbridge/conf_config_parser.c
M main/config_options.c
2 files changed, 8 insertions(+), 1 deletion(-)

Approvals:
  George Joseph: Approved for Submit
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved



diff --git a/apps/confbridge/conf_config_parser.c b/apps/confbridge/conf_config_parser.c
index c272a90..a182b51 100644
--- a/apps/confbridge/conf_config_parser.c
+++ b/apps/confbridge/conf_config_parser.c
@@ -2054,7 +2054,7 @@
 static int menu_template_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
 {
 	struct conf_menu *dst_menu = obj;
-	struct confbridge_cfg *cfg = aco_pending_config(&cfg_info);
+	RAII_VAR(struct confbridge_cfg *, cfg, ao2_global_obj_ref(cfg_handle), ao2_cleanup);
 	RAII_VAR(struct conf_menu *, src_menu, NULL, ao2_cleanup);
 
 	if (!cfg) {
@@ -2089,6 +2089,7 @@
 	RAII_VAR(struct user_profile *, user_profile, NULL, ao2_cleanup);
 	RAII_VAR(struct bridge_profile *, bridge_profile, NULL, ao2_cleanup);
 	RAII_VAR(struct conf_menu *, menu_profile, NULL, ao2_cleanup);
+	/* We can only be called as a result of an aco_process_config so this is safe */
 	struct confbridge_cfg *cfg = aco_pending_config(&cfg_info);
 
 	if (!cfg) {
@@ -2189,6 +2190,7 @@
 
 	/* Menu options */
 	aco_option_register(&cfg_info, "type", ACO_EXACT, menu_types, NULL, OPT_NOOP_T, 0, 0);
+	/* This option should only be used with the CONFBRIDGE dialplan function */
 	aco_option_register_custom(&cfg_info, "template", ACO_EXACT, menu_types, NULL, menu_template_handler, 0);
 	aco_option_register_custom(&cfg_info, "^[0-9A-D*#]+$", ACO_REGEX, menu_types, NULL, menu_option_handler, 0);
 
diff --git a/main/config_options.c b/main/config_options.c
index 2d23008..191e254 100644
--- a/main/config_options.c
+++ b/main/config_options.c
@@ -588,10 +588,13 @@
 	};
 
 	ao2_cleanup(info->internal->pending);
+	info->internal->pending = NULL;
 	return ACO_PROCESS_OK;
 
 error:
 	ao2_cleanup(info->internal->pending);
+	info->internal->pending = NULL;
+
 	return ACO_PROCESS_ERROR;
 }
 
@@ -704,6 +707,8 @@
 
 end:
 	ao2_cleanup(info->internal->pending);
+	info->internal->pending = NULL;
+
 	return res;
 }
 int aco_process_var(struct aco_type *type, const char *cat, struct ast_variable *var, void *obj)

-- 
To view, visit https://gerrit.asterisk.org/5580
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib349a17d3d088f092480b19addd7122fcaac21a7
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>