[asterisk-commits] twilson: trunk r370341 - in /trunk: apps/ apps/confbridge/ main/

[SVN commits to the Asterisk project]

Author: twilson
Date: Sat Jul 21 08:25:26 2012
New Revision: 370341

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=370341
Log:
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

Modified:
    trunk/apps/app_confbridge.c
    trunk/apps/confbridge/conf_config_parser.c
    trunk/main/config_options.c

Modified: trunk/apps/app_confbridge.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/app_confbridge.c?view=diff&rev=370341&r1=370340&r2=370341
==============================================================================
--- trunk/apps/app_confbridge.c (original)
+++ trunk/apps/app_confbridge.c Sat Jul 21 08:25:26 2012
@@ -2866,7 +2866,8 @@
 	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;
 }
 

Modified: trunk/apps/confbridge/conf_config_parser.c
URL: http://svnview.digium.com/svn/asterisk/trunk/apps/confbridge/conf_config_parser.c?view=diff&rev=370341&r1=370340&r2=370341
==============================================================================
--- trunk/apps/confbridge/conf_config_parser.c (original)
+++ trunk/apps/confbridge/conf_config_parser.c Sat Jul 21 08:25:26 2012
@@ -1183,6 +1183,8 @@
 		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 @@
 {
 	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 @@
 	/* 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 @@
 	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))) {

Modified: trunk/main/config_options.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/config_options.c?view=diff&rev=370341&r1=370340&r2=370341
==============================================================================
--- trunk/main/config_options.c (original)
+++ trunk/main/config_options.c Sat Jul 21 08:25:26 2012
@@ -606,10 +606,12 @@
 
 	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;