From 6b10de8830300cf41164e81d20ebdafca484d84c Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Fri, 3 Jun 2011 22:09:36 +0000 Subject: [PATCH] Asterisk crash when unloading cdr_radius/cel_radius. The rc_openlog() API call is passed a string that is used by openlog() to format log messages. The openlog() does not copy the string it just keeps a pointer to it. When the module is unloaded, the string is gone from memory. Depending upon module load order and if the other module then has an error, a crash happens. * Pass rc_openlog() a strdup'd string with the understanding that there will be a small memory leak if the cdr_radius/cel_radius modules are unloaded. * Call rc_destroy() to free the rc handle memory when the module is unloaded. JIRA AST-483 JIRA SWP-3062 git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@321926 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- cdr/cdr_radius.c | 28 ++++++++++++++++++++++++---- cel/cel_radius.c | 22 +++++++++++++++++++--- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/cdr/cdr_radius.c b/cdr/cdr_radius.c index 3a6190942f..bf613284dc 100644 --- a/cdr/cdr_radius.c +++ b/cdr/cdr_radius.c @@ -224,6 +224,10 @@ return_cleanup: static int unload_module(void) { ast_cdr_unregister(name); + if (rh) { + rc_destroy(rh); + rh = NULL; + } return 0; } @@ -243,8 +247,17 @@ static int load_module(void) } else return AST_MODULE_LOAD_DECLINE; - /* start logging */ - rc_openlog("asterisk"); + /* + * start logging + * + * NOTE: Yes this causes a slight memory leak if the module is + * unloaded. However, it is better than a crash if cdr_radius + * and cel_radius are both loaded. + */ + tmp = ast_strdup("asterisk"); + if (tmp) { + rc_openlog((char *) tmp); + } /* read radiusclient-ng config file */ if (!(rh = rc_read_config(radiuscfg))) { @@ -255,11 +268,18 @@ static int load_module(void) /* read radiusclient-ng dictionaries */ if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary"))) { ast_log(LOG_NOTICE, "Cannot load radiusclient-ng dictionary file.\n"); + rc_destroy(rh); + rh = NULL; return AST_MODULE_LOAD_DECLINE; } - ast_cdr_register(name, desc, radius_log); - return AST_MODULE_LOAD_SUCCESS; + if (ast_cdr_register(name, desc, radius_log)) { + rc_destroy(rh); + rh = NULL; + return AST_MODULE_LOAD_DECLINE; + } else { + return AST_MODULE_LOAD_SUCCESS; + } } AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_LOAD_ORDER, "RADIUS CDR Backend", diff --git a/cel/cel_radius.c b/cel/cel_radius.c index 0e20571d5e..5a4c837d97 100644 --- a/cel/cel_radius.c +++ b/cel/cel_radius.c @@ -208,6 +208,10 @@ static int unload_module(void) if (event_sub) { event_sub = ast_event_unsubscribe(event_sub); } + if (rh) { + rc_destroy(rh); + rh = NULL; + } return AST_MODULE_LOAD_SUCCESS; } @@ -227,8 +231,17 @@ static int load_module(void) return AST_MODULE_LOAD_DECLINE; } - /* start logging */ - rc_openlog("asterisk"); + /* + * start logging + * + * NOTE: Yes this causes a slight memory leak if the module is + * unloaded. However, it is better than a crash if cdr_radius + * and cel_radius are both loaded. + */ + tmp = ast_strdup("asterisk"); + if (tmp) { + rc_openlog((char *) tmp); + } /* read radiusclient-ng config file */ if (!(rh = rc_read_config(radiuscfg))) { @@ -239,12 +252,15 @@ static int load_module(void) /* read radiusclient-ng dictionaries */ if (rc_read_dictionary(rh, rc_conf_str(rh, "dictionary"))) { ast_log(LOG_NOTICE, "Cannot load radiusclient-ng dictionary file.\n"); + rc_destroy(rh); + rh = NULL; return AST_MODULE_LOAD_DECLINE; } event_sub = ast_event_subscribe(AST_EVENT_CEL, radius_log, "CEL Radius Logging", NULL, AST_EVENT_IE_END); - if (!event_sub) { + rc_destroy(rh); + rh = NULL; return AST_MODULE_LOAD_DECLINE; } else { return AST_MODULE_LOAD_SUCCESS; -- 2.47.2