From 13427db64c2c2f3cf15501c998bc982e91f8bf58 Mon Sep 17 00:00:00 2001
From: Terry Wilson <twilson@digium.com>
Date: Sat, 21 Jul 2012 13:25:26 +0000
Subject: [PATCH] Fix segfault introduced by conversion to ACO API
The value "none" is specified in the config file as a valid value for
the "video_mode" option. The code prior to the ACO conversion did not
check for "none", but just ignored it and relied on the default zero
value. The parsing with ACO is more strict, so without handling
"none" specifically, parsing would fail.
When parsing failed, but the module loaded anyway, the config info
would never be stored, and one place in the code did not check for
this case and would segfault. It was also possible that the
aco_info struct's internals would be destroyed and used as well.
This patch keeps the module from loading after parse failures, adds
the "none" option to "video_mode", registers CLI functions only
after parsing has completed, checks the config data for NULL before
accessing it, and returns -1 on some allocation failures when
initializing.
(closes issue ASTERISK-20159)
Reported by: Birger "WIMPy" Harzenetter
Tested by: Birger "WIMPy" Harzenetter
Patches:
confbridge_fix3.txt uploaded by Terry Wilson
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@370341 65c4cc65-6c06-0410-ace0-fbb531ad65f3
---
apps/app_confbridge.c | 3 ++-
apps/confbridge/conf_config_parser.c | 21 ++++++++++++++++-----
main/config_options.c | 2 ++
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/apps/app_confbridge.c b/apps/app_confbridge.c
index 9b1df3404e..2ea8d154e8 100644
--- a/apps/app_confbridge.c
+++ b/apps/app_confbridge.c
@@ -2866,7 +2866,8 @@ static int load_module(void)
res |= ast_manager_register_xml("ConfbridgeStopRecord", EVENT_FLAG_CALL, action_confbridgestoprecord);
res |= ast_manager_register_xml("ConfbridgeSetSingleVideoSrc", EVENT_FLAG_CALL, action_confbridgesetsinglevideosrc);
- conf_load_config(0);
+ res |= conf_load_config(0);
+
return res;
}
diff --git a/apps/confbridge/conf_config_parser.c b/apps/confbridge/conf_config_parser.c
index dc81e3cc85..f4a9604107 100644
--- a/apps/confbridge/conf_config_parser.c
+++ b/apps/confbridge/conf_config_parser.c
@@ -1183,6 +1183,8 @@ static int video_mode_handler(const struct aco_option *opt, struct ast_variable
ast_set_flag(b_profile, BRIDGE_OPT_VIDEO_SRC_LAST_MARKED);
} else if (!strcasecmp(var->value, "follow_talker")) {
ast_set_flag(b_profile, BRIDGE_OPT_VIDEO_SRC_FOLLOW_TALKER);
+ } else if (!strcasecmp(var->value, "none")) {
+ return 0;
} else {
return -1;
}
@@ -1255,10 +1257,7 @@ int conf_load_config(int reload)
{
if (!reload) {
if (aco_info_init(&cfg_info)) {
- goto error;
- }
- if (ast_cli_register_multiple(cli_confbridge_parser, ARRAY_LEN(cli_confbridge_parser))) {
- goto error;
+ return -1;
}
}
@@ -1307,7 +1306,15 @@ int conf_load_config(int reload)
/* Menu options */
aco_option_register_custom(&cfg_info, "^[0-9A-D*#]+$", ACO_REGEX, menu_types, NULL, menu_option_handler, 0);
- return aco_process_config(&cfg_info, reload) == ACO_PROCESS_ERROR;
+ if (aco_process_config(&cfg_info, reload) == ACO_PROCESS_ERROR) {
+ goto error;
+ }
+
+ if (!reload && ast_cli_register_multiple(cli_confbridge_parser, ARRAY_LEN(cli_confbridge_parser))) {
+ goto error;
+ }
+
+ return 0;
error:
conf_destroy_config();
return -1;
@@ -1380,6 +1387,10 @@ const struct bridge_profile *conf_find_bridge_profile(struct ast_channel *chan,
struct func_confbridge_data *b_data = NULL;
RAII_VAR(struct confbridge_cfg *, cfg, ao2_global_obj_ref(cfg_handle), ao2_cleanup);
+ if (!cfg) {
+ return NULL;
+ }
+
if (chan) {
ast_channel_lock(chan);
if ((datastore = ast_channel_datastore_find(chan, &confbridge_datastore, NULL))) {
diff --git a/main/config_options.c b/main/config_options.c
index a958342ec6..1801f93dd5 100644
--- a/main/config_options.c
+++ b/main/config_options.c
@@ -606,10 +606,12 @@ static int internal_type_init(struct aco_type *type)
if (!(type->internal->regex = build_regex(type->category))) {
internal_type_destroy(type);
+ return -1;
}
if (!(type->internal->opts = aco_option_container_alloc())) {
internal_type_destroy(type);
+ return -1;
}
return 0;
--
GitLab