DeforaNetworks/fwknop
3
0
Fork 0
Code Releases Activity

This commit fixes two memory leaks and adds a common exit function.

Browse Source 
The two memory leaks were found with the test suite running in
--enable-valgrind mode - here are the relevant error messages:

For fwknopd server GPG clean up:

==345== 9 bytes in 1 blocks are definitely lost in loss record 2 of 2
==345==   at 0x4C2815C: malloc (vg_replace_malloc.c:236)
==345==   by 0x52F6B81: strdup (strdup.c:43)
==345==   by 0x10FA57: add_string_list_ent (access.c:308)
==345==   by 0x110513: parse_access_file (access.c:387)
==345==   by 0x10B5FB: main (fwknopd.c:193)

For fwknop client rc file processing:

==8045== 568 bytes in 1 blocks are still reachable in loss record 12 of 12
==8045==    at 0x4C2815C: malloc (vg_replace_malloc.c:236)
==8045==    by 0x50A53AA: __fopen_internal (iofopen.c:76)
==8045==    by 0x10C3FF: process_rc (config_init.c:446)
==8045==    by 0x10C8F6: config_init (config_init.c:671)
==8045==    by 0x10AC9E: main (fwknop.c:62)

There is also a new clean_exit() function that makes it easier to ensure that
resources are deallocated upon existing.
This commit is contained in:
Michael Rash
2011-11-10 22:33:32 -05:00
parent 9ebd55f522
commit 50b48147c0
15 changed files with 79 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/config_init.c
+2 -1
View File
@@ -453,7 +453,7 @@ process_rc(fko_cli_options_t *options)
else
fprintf(stderr, "Unable to open rc file: %s: %s\n",
rcfile, strerror(errno));
return;
}
@@ -548,6 +548,7 @@ process_rc(fko_cli_options_t *options)
}
} /* end while fgets rc */
fclose(rc);
}
/* Sanity and bounds checks for the various options.
client/fwknop.c
+9
View File
@@ -361,9 +361,18 @@ main(int argc, char **argv)
fko_destroy(ctx);
free_configs(&options);
return(EXIT_SUCCESS);
}
void
free_configs(fko_cli_options_t *opts)
{
if (opts->resolve_url != NULL)
free(opts->resolve_url);
}
static int
get_rand_port(fko_ctx_t ctx)
{
client/fwknop_common.h
+2
View File
@@ -137,6 +137,8 @@ typedef struct fko_cli_options
extern fko_cli_options_t options;
void free_configs(fko_cli_options_t *opts);
#endif /* FWKNOP_COMMON_H */
/***EOF***/
lib/fko_user.c
-1
View File
@@ -95,7 +95,6 @@ fko_set_username(fko_ctx_t ctx, const char *spoof_user)
if(ctx->username == NULL)
return(FKO_ERROR_MEMORY_ALLOCATION);
return(FKO_SUCCESS);
}
server/access.c
+11 -10
View File
@@ -431,6 +431,7 @@ free_acc_string_list(acc_string_list_t *stl)
last_stl = stl;
stl = last_stl->next;
free(last_stl->str);
free(last_stl);
}
}
@@ -571,7 +572,7 @@ acc_stanza_add(fko_srv_options_t *opts)
log_msg(LOG_ERR,
"Fatal memory allocation error adding access stanza"
);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* If this is not the first acc entry, we walk our acc pointer to the
@@ -664,7 +665,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr, "[*] Access file: '%s' was not found.\n",
opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
if ((file_ptr = fopen(opts->config[CONF_ACCESS_FILE], "r")) == NULL)
@@ -673,7 +674,7 @@ parse_access_file(fko_srv_options_t *opts)
opts->config[CONF_ACCESS_FILE]);
perror(NULL);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Initialize the access list.
@@ -733,7 +734,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr,
"[*] Data error in access file: '%s'\n",
opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
@@ -766,7 +767,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr,
"[*] KEY value is not properly set in stanza source '%s' in access file: '%s'\n",
curr_acc->source, opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
add_acc_string(&(curr_acc->key), val);
}
@@ -789,7 +790,7 @@ parse_access_file(fko_srv_options_t *opts)
{
fprintf(stderr, "Unable to determine UID for CMD_EXEC_USER: %s.\n",
errno ? strerror(errno) : "Not a user on this system");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
curr_acc->cmd_exec_uid = pw->pw_uid;
@@ -813,7 +814,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr,
"[*] GPG_HOME_DIR directory '%s' stat()/existence problem in stanza source '%s' in access file: '%s'\n",
val, curr_acc->source, opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
else if(CONF_VAR_IS(var, "GPG_DECRYPT_ID"))
@@ -827,7 +828,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr,
"[*] GPG_DECRYPT_PW value is not properly set in stanza source '%s' in access file: '%s'\n",
curr_acc->source, opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
add_acc_string(&(curr_acc->gpg_decrypt_pw), val);
}
@@ -863,7 +864,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr,
"[*] Could not find valid SOURCE stanza in access file: '%s'\n",
opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Sanity check the last stanza
@@ -873,7 +874,7 @@ parse_access_file(fko_srv_options_t *opts)
fprintf(stderr,
"[*] Data error in access file: '%s'\n",
opts->config[CONF_ACCESS_FILE]);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Expand our the expandable fields into their respective data buckets.
server/config_init.c
+10 -11
View File
@@ -48,7 +48,7 @@ set_config_entry(fko_srv_options_t *opts, const int var_ndx, const char *value)
if(var_ndx < 0 || var_ndx >= NUMBER_OF_CONFIG_ENTRIES)
{
fprintf(stderr, "Index value of %i is not valid\n", var_ndx);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* If this particular entry was already set (i.e. not NULL), then
@@ -74,7 +74,7 @@ set_config_entry(fko_srv_options_t *opts, const int var_ndx, const char *value)
if(opts->config[var_ndx] == NULL)
{
fprintf(stderr, "*Fatal memory allocation error!\n");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
strlcpy(opts->config[var_ndx], value, space_needed);
@@ -135,8 +135,7 @@ parse_config_file(fko_srv_options_t *opts, const char *config_file)
{
fprintf(stderr, "[*] Config file: '%s' was not found.\n",
config_file);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
if ((cfile_ptr = fopen(config_file, "r")) == NULL)
@@ -145,7 +144,7 @@ parse_config_file(fko_srv_options_t *opts, const char *config_file)
config_file);
perror(NULL);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
while ((fgets(conf_line_buf, MAX_LINE_LEN, cfile_ptr)) != NULL)
@@ -498,7 +497,7 @@ validate_options(fko_srv_options_t *opts)
fprintf(stderr,
"The -D, -K, -R, and -S options are mutually exclusive. Pick only one.\n"
);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
if(opts->config[CONF_FIREWALL_EXE] == NULL)
@@ -506,7 +505,7 @@ validate_options(fko_srv_options_t *opts)
fprintf(stderr,
"No firewall command executable is set. Please check FIREWALL_EXE in fwknopd.conf.\n"
);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
return;
@@ -566,7 +565,7 @@ config_init(fko_srv_options_t *opts, int argc, char **argv)
switch(cmd_arg) {
case 'h':
usage();
exit(EXIT_SUCCESS);
clean_exit(opts, NO_FW_CLEANUP, EXIT_SUCCESS);
break;
/* Look for configuration file arg.
@@ -692,7 +691,7 @@ config_init(fko_srv_options_t *opts, int argc, char **argv)
fprintf(stderr,
"[*] Directory '%s' could not stat()/does not exist?\n",
optarg);
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
break;
case 'i':
@@ -727,11 +726,11 @@ config_init(fko_srv_options_t *opts, int argc, char **argv)
break;
case 'V':
fprintf(stdout, "fwknopd server %s\n", MY_VERSION);
exit(EXIT_SUCCESS);
clean_exit(opts, NO_FW_CLEANUP, EXIT_SUCCESS);
break;
default:
usage();
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
server/fw_util_ipf.c
+1 -1
View File
@@ -91,7 +91,7 @@ fw_initialize(const fko_srv_options_t *opts)
if(res != 0)
{
fprintf(stderr, "Warning: Errors detected during fwknop custom chain creation.\n");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
server/fw_util_ipfw.c
+2 -2
View File
@@ -217,7 +217,7 @@ fw_initialize(const fko_srv_options_t *opts)
if(res != 0)
{
fprintf(stderr, "Fatal: Errors detected during ipfw rules initialization.\n");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Allocate our rule_map array for tracking active (and expired) rules.
@@ -227,7 +227,7 @@ fw_initialize(const fko_srv_options_t *opts)
if(fwc.rule_map == NULL)
{
fprintf(stderr, "Fatal: Memory allocation error in fw_initialize.\n");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Create a check-state rule if necessary.
server/fw_util_pf.c
+1 -1
View File
@@ -182,7 +182,7 @@ fw_initialize(const fko_srv_options_t *opts)
if (! anchor_active(opts))
{
fprintf(stderr, "Warning: the fwknop anchor is not active in the pf policy\n");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Delete any existing rules in the fwknop anchor
server/fwknopd.c
+26 -12
View File
@@ -78,18 +78,18 @@ main(int argc, char **argv)
if(res == 0)
{
fprintf(stdout, "Killed fwknopd (pid=%i)\n", old_pid);
exit(EXIT_SUCCESS);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_SUCCESS);
}
else
{
perror("Unable to kill fwknop: ");
exit(EXIT_FAILURE);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
else
{
fprintf(stderr, "No running fwknopd detected.\n");
exit(EXIT_FAILURE);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
@@ -104,7 +104,7 @@ main(int argc, char **argv)
else
fprintf(stdout, "No running fwknopd detected.\n");
exit(EXIT_SUCCESS);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_SUCCESS);
}
/* Restart the currently running fwknopd?
@@ -119,18 +119,18 @@ main(int argc, char **argv)
if(res == 0)
{
fprintf(stdout, "Sent restart signal to fwknopd (pid=%i)\n", old_pid);
exit(EXIT_SUCCESS);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_SUCCESS);
}
else
{
perror("Unable to send signal to fwknop: ");
exit(EXIT_FAILURE);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
else
{
fprintf(stdout, "No running fwknopd detected.\n");
exit(EXIT_FAILURE);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
@@ -179,14 +179,13 @@ main(int argc, char **argv)
if(opts.fw_list == 1 || opts.fw_list_all == 1)
{
fw_dump_rules(&opts);
exit(EXIT_SUCCESS);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_SUCCESS);
}
if(opts.fw_flush == 1)
{
fprintf(stdout, "Deleting any existing firewall rules...\n");
fw_cleanup(&opts);
exit(EXIT_SUCCESS);
clean_exit(&opts, FW_CLEANUP, EXIT_SUCCESS);
}
/* Process the access.conf file.
@@ -200,7 +199,7 @@ main(int argc, char **argv)
{
dump_config(&opts);
dump_access_list(&opts);
exit(EXIT_SUCCESS);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_SUCCESS);
}
/* If we are a new process (just being started), proceed with normal
@@ -225,7 +224,7 @@ main(int argc, char **argv)
"* An instance of fwknopd is already running: (PID=%i).\n", old_pid
);
exit(EXIT_FAILURE);
clean_exit(&opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
else if(old_pid < 0)
{
@@ -681,4 +680,19 @@ get_running_pid(const fko_srv_options_t *opts)
return(rpid);
}
void
clean_exit(fko_srv_options_t *opts, unsigned int fw_cleanup_flag, unsigned int exit_status)
{
if(fw_cleanup_flag == FW_CLEANUP)
fw_cleanup(opts);
#if USE_FILE_CACHE
free_replay_list(opts);
#endif
free_logging();
free_configs(opts);
exit(exit_status);
}
/***EOF***/
server/fwknopd_common.h
+6
View File
@@ -444,6 +444,12 @@ typedef struct fko_srv_options
extern fko_srv_options_t options;
/* For cleaning up memory before exiting
*/
#define FW_CLEANUP 1
#define NO_FW_CLEANUP 0
void clean_exit(fko_srv_options_t *opts, unsigned int fw_cleanup_flag, unsigned int exit_status);
#endif /* FWKNOPD_COMMON_H */
/***EOF***/
server/incoming_spa.c
+1
View File
@@ -279,6 +279,7 @@ incoming_spa(fko_srv_options_t *opts)
/* Now decrypt the data.
*/
res = fko_decrypt_spa_data(ctx, acc->gpg_decrypt_pw);
}
else
{
server/log_msg.c
+1 -1
View File
@@ -87,7 +87,7 @@ init_logging(fko_srv_options_t *opts) {
if(log_name == NULL)
{
fprintf(stderr, "Memory allocation error setting log_name!\n");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Set our name.
server/pcap_capture.c
+6 -6
View File
@@ -78,13 +78,13 @@ pcap_capture(fko_srv_options_t *opts)
if(pcap == NULL)
{
log_msg(LOG_ERR, "[*] pcap_open_live error: %s\n", errstr);
exit(EXIT_FAILURE);
clean_exit(opts, FW_CLEANUP, EXIT_FAILURE);
}
if (pcap == NULL)
{
log_msg(LOG_ERR, "[*] pcap error: %s", errstr);
exit(EXIT_FAILURE);
clean_exit(opts, FW_CLEANUP, EXIT_FAILURE);
}
/* Set pcap filters, if any.
@@ -96,7 +96,7 @@ pcap_capture(fko_srv_options_t *opts)
log_msg(LOG_ERR, "[*] Error compiling pcap filter: %s",
pcap_geterr(pcap)
);
exit(EXIT_FAILURE);
clean_exit(opts, FW_CLEANUP, EXIT_FAILURE);
}
if(pcap_setfilter(pcap, &fp) == -1)
@@ -104,7 +104,7 @@ pcap_capture(fko_srv_options_t *opts)
log_msg(LOG_ERR, "[*] Error setting pcap filter: %s",
pcap_geterr(pcap)
);
exit(EXIT_FAILURE);
clean_exit(opts, FW_CLEANUP, EXIT_FAILURE);
}
log_msg(LOG_INFO, "PCAP filter is: %s", opts->config[CONF_PCAP_FILTER]);
@@ -154,7 +154,7 @@ pcap_capture(fko_srv_options_t *opts)
log_msg(LOG_ERR, "[*] Error setting pcap nonblocking to %i: %s",
0, errstr
);
exit(EXIT_FAILURE);
clean_exit(opts, FW_CLEANUP, EXIT_FAILURE);
}
/* Initialize our signal handlers. You can check the return value for
@@ -257,7 +257,7 @@ pcap_capture(fko_srv_options_t *opts)
log_msg(LOG_ERR, "[*] %i consecutive pcap errors. Giving up",
pcap_errcnt
);
exit(EXIT_FAILURE);
clean_exit(opts, FW_CLEANUP, EXIT_FAILURE);
}
}
else if(pending_break == 1 || res == -2)
server/replay_cache.c
+1 -1
View File
@@ -99,7 +99,7 @@ rotate_digest_cache_file(fko_srv_options_t *opts)
if(new_file == NULL)
{
log_msg(LOG_ERR, "rotate_digest_cache_file: Memory allocation error.");
exit(EXIT_FAILURE);
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* The new filename is just the original with a trailing '-old'.