[asterisk-commits] elguero: branch 1.8 r385551 - /branches/1.8/apps/app_voicemail.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Apr 12 17:14:35 CDT 2013
Author: elguero
Date: Fri Apr 12 17:14:31 2013
New Revision: 385551
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=385551
Log:
Fix app_voicemail Segfault And A Few Memory Leaks
The original report was that app_voicemail would crash. This was caused by
ast_config_load() returning CONFIG_STATUS_FILEINVALID but no checks being
performed for that return status. After adding the initial patch to fix this
issue, Jaco Kroon (jkroon) added some fixes to memory leaks he had discovered.
During review, Walter Doekes (wdoekes) suggested adding a helper function in
order to determine if we had a valid configuration or not.
This patch does the following:
* Creates a helper function to check if the configuration is valid
* Adds calls to the new helper function where appropiate
* Fixes memory leaks where the code returned without running
ast_config_destroy() on the configuration that was loaded
(closes issue ASTERISK-21302)
Reported by: Jaco Kroon
Tested by: Jaco Kroon, Michael L. Young
Patches:
asterisk-11.3.0-app_voicemail-ast_config-fixes.patch
Jaco Kroon (license 5671)
asterisk-21302-valid_cfg_and_mem_leaks_v3-1.8.diff
Michael L. Young (license 5026)
Review: https://reviewboard.asterisk.org/r/2443/
Modified:
branches/1.8/apps/app_voicemail.c
Modified: branches/1.8/apps/app_voicemail.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/apps/app_voicemail.c?view=diff&rev=385551&r1=385550&r2=385551
==============================================================================
--- branches/1.8/apps/app_voicemail.c (original)
+++ branches/1.8/apps/app_voicemail.c Fri Apr 12 17:14:31 2013
@@ -1507,6 +1507,14 @@
return res;
}
+/*!
+ * \brief Check if configuration file is valid
+ */
+static inline int valid_config(const struct ast_config *cfg)
+{
+ return cfg && cfg != CONFIG_STATUS_FILEINVALID;
+}
+
/*!
* \brief The handler for the change password option.
* \param vmu The voicemail user to work with.
@@ -1543,7 +1551,7 @@
}
/* Fall-through */
case OPT_PWLOC_VOICEMAILCONF:
- if ((cfg = ast_config_load(VOICEMAIL_CONFIG, config_flags)) && cfg != CONFIG_STATUS_FILEINVALID) {
+ if ((cfg = ast_config_load(VOICEMAIL_CONFIG, config_flags)) && valid_config(cfg)) {
while ((category = ast_category_browse(cfg, category))) {
if (!strcasecmp(category, vmu->context)) {
if (!(tmp = ast_variable_retrieve(cfg, category, vmu->mailbox))) {
@@ -1572,14 +1580,17 @@
reset_user_pw(vmu->context, vmu->mailbox, newpassword);
ast_copy_string(vmu->password, newpassword, sizeof(vmu->password));
ast_config_text_file_save(VOICEMAIL_CONFIG, cfg, "AppVoicemail");
+ ast_config_destroy(cfg);
break;
}
+
+ ast_config_destroy(cfg);
}
/* Fall-through */
case OPT_PWLOC_USERSCONF:
/* check users.conf and update the password stored for the mailbox */
/* if no vmsecret entry exists create one. */
- if ((cfg = ast_config_load("users.conf", config_flags)) && cfg != CONFIG_STATUS_FILEINVALID) {
+ if ((cfg = ast_config_load("users.conf", config_flags)) && valid_config(cfg)) {
ast_debug(4, "we are looking for %s\n", vmu->mailbox);
for (category = ast_category_browse(cfg, NULL); category; category = ast_category_browse(cfg, category)) {
ast_debug(4, "users.conf: %s\n", category);
@@ -1613,6 +1624,8 @@
ast_copy_string(vmu->password, newpassword, sizeof(vmu->password));
ast_config_text_file_save("users.conf", cfg, "AppVoicemail");
}
+
+ ast_config_destroy(cfg);
}
}
}
@@ -3846,7 +3859,7 @@
res = -1;
break;
}
- if (cfg && cfg != CONFIG_STATUS_FILEINVALID) {
+ if (valid_config(cfg)) {
if (!(idata.context = ast_variable_retrieve(cfg, "message", "context"))) {
idata.context = "";
}
@@ -3899,7 +3912,7 @@
if (obj) {
ast_odbc_release_obj(obj);
}
- if (cfg)
+ if (valid_config(cfg))
ast_config_destroy(cfg);
if (fdm != MAP_FAILED)
munmap(fdm, fdlen);
@@ -4378,7 +4391,7 @@
if (strlen(fromfile) < sizeof(fromfile) - 5) {
strcat(fromfile, ".txt");
}
- if (!(msg_cfg = ast_config_load(fromfile, config_flags))) {
+ if (!(msg_cfg = ast_config_load(fromfile, config_flags)) || !(valid_config(msg_cfg))) {
if (option_debug > 0) {
ast_log(LOG_DEBUG, "Config load for message text file '%s' failed\n", fromfile);
}
@@ -4753,7 +4766,7 @@
if (strlen(fromfile) < sizeof(fromfile) - 5) {
strcat(fromfile, ".txt");
}
- if ((msg_cfg = ast_config_load(fromfile, config_flags))) {
+ if ((msg_cfg = ast_config_load(fromfile, config_flags)) && valid_config(msg_cfg)) {
if ((v = ast_variable_retrieve(msg_cfg, "message", "callerid"))) {
ast_copy_string(origcallerid, v, sizeof(origcallerid));
}
@@ -6904,7 +6917,7 @@
strncat(backup, "-bak", sizeof(backup) - strlen(backup) - 1);
strncat(backup_textfile, "-bak.txt", sizeof(backup_textfile) - strlen(backup_textfile) - 1);
- if ((msg_cfg = ast_config_load(textfile, config_flags)) && msg_cfg != CONFIG_STATUS_FILEINVALID && (duration_str = ast_variable_retrieve(msg_cfg, "message", "duration"))) {
+ if ((msg_cfg = ast_config_load(textfile, config_flags)) && valid_config(msg_cfg) && (duration_str = ast_variable_retrieve(msg_cfg, "message", "duration"))) {
*duration = atoi(duration_str);
} else {
*duration = 0;
@@ -6937,7 +6950,7 @@
*duration = 0;
/* if we can't read the message metadata, stop now */
- if (!msg_cfg) {
+ if (!valid_config(msg_cfg)) {
cmd = 0;
break;
}
@@ -7021,7 +7034,7 @@
}
}
- if (msg_cfg)
+ if (valid_config(msg_cfg))
ast_config_destroy(msg_cfg);
if (prepend_duration)
*duration = prepend_duration;
@@ -7717,7 +7730,7 @@
snprintf(filename, sizeof(filename), "%s.txt", vms->fn);
RETRIEVE(vms->curdir, vms->curmsg, vmu->mailbox, vmu->context);
msg_cfg = ast_config_load(filename, config_flags);
- if (!msg_cfg || msg_cfg == CONFIG_STATUS_FILEINVALID) {
+ if (!valid_config(msg_cfg)) {
ast_log(LOG_WARNING, "No message attribute file?!! (%s)\n", filename);
return 0;
}
@@ -7795,7 +7808,7 @@
}
}
- if (!msg_cfg) {
+ if (!valid_config(msg_cfg)) {
ast_log(AST_LOG_WARNING, "No message attribute file?!! (%s)\n", filename);
return 0;
}
@@ -12573,7 +12586,7 @@
struct ast_flags config_flags = { 0 };
pwconf = ast_config_load(secretfn, config_flags);
- if (pwconf) {
+ if (valid_config(pwconf)) {
const char *val = ast_variable_retrieve(pwconf, "general", "password");
if (val) {
ast_copy_string(password, val, passwordlen);
@@ -13058,7 +13071,7 @@
fputs("00000002 => 9999,Mrs. Test\n", file);
fclose(file);
- if (!(cfg = ast_config_load(config_filename, config_flags))) {
+ if (!(cfg = ast_config_load(config_filename, config_flags)) || !valid_config(cfg)) {
res = AST_TEST_FAIL;
goto cleanup;
}
@@ -13279,7 +13292,7 @@
RETRIEVE(vms->curdir, vms->curmsg, vmu->mailbox, vmu->context);
msg_cfg = ast_config_load(filename, config_flags);
DISPOSE(vms->curdir, vms->curmsg);
- if (!msg_cfg || msg_cfg == CONFIG_STATUS_FILEINVALID) {
+ if (!valid_config(msg_cfg)) {
ast_log(AST_LOG_WARNING, "No message attribute file?!! (%s)\n", filename);
return 0;
}
@@ -13439,9 +13452,9 @@
break;
}
+ ast_config_destroy(msg_cfg);
+
#ifndef IMAP_STORAGE
- ast_config_destroy(msg_cfg);
-
if (!res) {
make_file(vms->fn, sizeof(vms->fn), vms->curdir, msg);
vms->heard[msg] = 1;
More information about the asterisk-commits
mailing list