mirror of
				https://github.com/asterisk/asterisk.git
				synced 2025-10-31 18:55:19 +00:00 
			
		
		
		
	Fix lock usage in cdr_sqlite3_custom to avoid potential crashes during reload.
Pointed out by Russell while working on the CEL branch. git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@202417 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
		| @@ -61,8 +61,6 @@ static const char name[] = "cdr_sqlite3_custom"; | ||||
| static sqlite3 *db = NULL; | ||||
|  | ||||
| static char table[80]; | ||||
|  | ||||
| /*! XXX \bug Handling of this variable has crash potential on reload */ | ||||
| static char *columns; | ||||
|  | ||||
| struct values { | ||||
| @@ -72,7 +70,7 @@ struct values { | ||||
|  | ||||
| static AST_LIST_HEAD_STATIC(sql_values, values); | ||||
|  | ||||
| static int free_config(void); | ||||
| static void free_config(void); | ||||
|  | ||||
| static int load_column_config(const char *tmp) | ||||
| { | ||||
| @@ -168,11 +166,8 @@ static int load_config(int reload) | ||||
| 		free_config(); | ||||
| 	} | ||||
|  | ||||
| 	ast_mutex_lock(&lock); | ||||
|  | ||||
| 	if (!(mappingvar = ast_variable_browse(cfg, "master"))) { | ||||
| 		/* Nothing configured */ | ||||
| 		ast_mutex_unlock(&lock); | ||||
| 		ast_config_destroy(cfg); | ||||
| 		return -1; | ||||
| 	} | ||||
| @@ -187,7 +182,6 @@ static int load_config(int reload) | ||||
|  | ||||
| 	/* Columns */ | ||||
| 	if (load_column_config(ast_variable_retrieve(cfg, "master", "columns"))) { | ||||
| 		ast_mutex_unlock(&lock); | ||||
| 		ast_config_destroy(cfg); | ||||
| 		free_config(); | ||||
| 		return -1; | ||||
| @@ -195,7 +189,6 @@ static int load_config(int reload) | ||||
|  | ||||
| 	/* Values */ | ||||
| 	if (load_values_config(ast_variable_retrieve(cfg, "master", "values"))) { | ||||
| 		ast_mutex_unlock(&lock); | ||||
| 		ast_config_destroy(cfg); | ||||
| 		free_config(); | ||||
| 		return -1; | ||||
| @@ -203,18 +196,15 @@ static int load_config(int reload) | ||||
|  | ||||
| 	ast_verb(3, "cdr_sqlite3_custom: Logging CDR records to table '%s' in 'master.db'\n", table); | ||||
|  | ||||
| 	ast_mutex_unlock(&lock); | ||||
| 	ast_config_destroy(cfg); | ||||
|  | ||||
| 	return 0; | ||||
| } | ||||
|  | ||||
| static int free_config(void) | ||||
| static void free_config(void) | ||||
| { | ||||
| 	struct values *value; | ||||
|  | ||||
| 	ast_mutex_lock(&lock); | ||||
|  | ||||
| 	if (db) { | ||||
| 		sqlite3_close(db); | ||||
| 		db = NULL; | ||||
| @@ -228,10 +218,6 @@ static int free_config(void) | ||||
| 	while ((value = AST_LIST_REMOVE_HEAD(&sql_values, list))) { | ||||
| 		ast_free(value); | ||||
| 	} | ||||
|  | ||||
| 	ast_mutex_unlock(&lock); | ||||
|  | ||||
| 	return 0; | ||||
| } | ||||
|  | ||||
| static int sqlite3_log(struct ast_cdr *cdr) | ||||
| @@ -246,6 +232,8 @@ static int sqlite3_log(struct ast_cdr *cdr) | ||||
| 		return 0; | ||||
| 	} | ||||
|  | ||||
| 	ast_mutex_lock(&lock); | ||||
|  | ||||
| 	{ /* Make it obvious that only sql should be used outside of this block */ | ||||
| 		char *escaped; | ||||
| 		char subst_buf[2048]; | ||||
| @@ -257,6 +245,7 @@ static int sqlite3_log(struct ast_cdr *cdr) | ||||
| 		if (!dummy) { | ||||
| 			ast_log(LOG_ERROR, "Unable to allocate channel for variable subsitution.\n"); | ||||
| 			ast_free(value_string); | ||||
| 			ast_mutex_unlock(&lock); | ||||
| 			return 0; | ||||
| 		} | ||||
| 		dummy->cdr = ast_cdr_dup(cdr); | ||||
| @@ -272,8 +261,6 @@ static int sqlite3_log(struct ast_cdr *cdr) | ||||
| 		ast_free(value_string); | ||||
| 	} | ||||
|  | ||||
| 	ast_mutex_lock(&lock); | ||||
|  | ||||
| 	/* XXX This seems awful arbitrary... */ | ||||
| 	for (count = 0; count < 5; count++) { | ||||
| 		res = sqlite3_exec(db, sql, NULL, NULL, &error); | ||||
| @@ -299,10 +286,10 @@ static int sqlite3_log(struct ast_cdr *cdr) | ||||
|  | ||||
| static int unload_module(void) | ||||
| { | ||||
| 	free_config(); | ||||
|  | ||||
| 	ast_cdr_unregister(name); | ||||
|  | ||||
| 	free_config(); | ||||
|  | ||||
| 	return 0; | ||||
| } | ||||
|  | ||||
| @@ -313,14 +300,7 @@ static int load_module(void) | ||||
| 	int res; | ||||
| 	char *sql; | ||||
|  | ||||
| 	if (!load_config(0)) { | ||||
| 		res = ast_cdr_register(name, desc, sqlite3_log); | ||||
| 		if (res) { | ||||
| 			ast_log(LOG_ERROR, "Unable to register custom SQLite3 CDR handling\n"); | ||||
| 			free_config(); | ||||
| 			return AST_MODULE_LOAD_DECLINE; | ||||
| 		} | ||||
| 	} else { | ||||
| 	if (load_config(0)) { | ||||
| 		return AST_MODULE_LOAD_DECLINE; | ||||
| 	} | ||||
|  | ||||
| @@ -350,12 +330,25 @@ static int load_module(void) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	return 0; | ||||
| 	res = ast_cdr_register(name, desc, sqlite3_log); | ||||
| 	if (res) { | ||||
| 		ast_log(LOG_ERROR, "Unable to register custom SQLite3 CDR handling\n"); | ||||
| 		free_config(); | ||||
| 		return AST_MODULE_LOAD_DECLINE; | ||||
| 	} | ||||
|  | ||||
| 	return AST_MODULE_LOAD_SUCCESS; | ||||
| } | ||||
|  | ||||
| static int reload(void) | ||||
| { | ||||
| 	return load_config(1); | ||||
| 	int res = 0; | ||||
|  | ||||
| 	ast_mutex_lock(&lock); | ||||
| 	res = load_config(1); | ||||
| 	ast_mutex_unlock(&lock); | ||||
|  | ||||
| 	return res; | ||||
| } | ||||
|  | ||||
| AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT, "SQLite3 Custom CDR Module", | ||||
|   | ||||
		Reference in New Issue
	
	Block a user