From c2d7633440a4bb979bc33532273b0dc1e9006d9e Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 8 Apr 2014 09:33:39 -0400 Subject: [PATCH 1/3] serval-dna: fix memory leak and error reporting to client --- plugins/serval-dna/crypto.c | 5 ++--- plugins/serval-dna/serval-dna.h | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/plugins/serval-dna/crypto.c b/plugins/serval-dna/crypto.c index 4a9547d..b8e8c07 100644 --- a/plugins/serval-dna/crypto.c +++ b/plugins/serval-dna/crypto.c @@ -234,7 +234,7 @@ svl_crypto_ctx_free(svl_crypto_ctx *ctx) { if (!ctx) return; - if (ctx->keyring_file) + if (ctx->keyring_file && ctx->keyring_file != keyring) keyring_free(ctx->keyring_file); h_free(ctx); } @@ -531,12 +531,11 @@ serval_crypto_handler(co_obj_t *self, co_obj_t **output, co_obj_t *params) } - return 1; error: INS_ERROR(); if (ctx) svl_crypto_ctx_free(ctx); - return 0; + return 1; } int diff --git a/plugins/serval-dna/serval-dna.h b/plugins/serval-dna/serval-dna.h index fadd56a..da589fc 100644 --- a/plugins/serval-dna/serval-dna.h +++ b/plugins/serval-dna/serval-dna.h @@ -44,7 +44,7 @@ err_msg = co_list16_create(); \ char *msg = NULL; \ int len = snprintf(NULL, 0, M, ##__VA_ARGS__); \ - msg = calloc(len,sizeof(char)); \ + msg = calloc(len + 1,sizeof(char)); \ sprintf(msg, M, ##__VA_ARGS__); \ if (len < UINT8_MAX) { \ co_list_append(err_msg, co_str8_create(msg,len+1,0)); \ @@ -65,7 +65,7 @@ co_obj_free(err_msg); \ char *msg = NULL; \ int len = snprintf(NULL, 0, M, ##__VA_ARGS__); \ - msg = calloc(len,sizeof(char)); \ + msg = calloc(len + 1,sizeof(char)); \ sprintf(msg, M, ##__VA_ARGS__); \ if (len < UINT8_MAX) { \ err_msg = co_str8_create(msg,len+1,0); \ @@ -78,7 +78,7 @@ goto error; \ } -#define CLEAR_ERR() if (err_msg) { co_obj_free(err_msg); err_msg = NULL; } +#define CLEAR_ERR() err_msg = NULL; #define INS_ERROR() if (err_msg) { CMD_OUTPUT("errors",err_msg); } From 4a95d7883bcb8ddfb2737dd9ad63575ede6bb96b Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Mon, 14 Apr 2014 15:45:33 -0400 Subject: [PATCH 2/3] fix memory leaks in commotiond --- src/daemon.c | 1 + src/iface.c | 4 ++++ src/iface.h | 5 +++++ src/profile.c | 29 +++++++++++++++++++++-------- src/profile.h | 6 +++++- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index 9fffa9e..bed7315 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -1022,6 +1022,7 @@ int main(int argc, char *argv[]) { co_cmds_shutdown(); co_profiles_shutdown(); co_plugins_shutdown(); + co_ifaces_shutdown(); return 0; } diff --git a/src/iface.c b/src/iface.c index dc11893..b6a31be 100644 --- a/src/iface.c +++ b/src/iface.c @@ -159,6 +159,10 @@ int co_ifaces_create(void) { return 0; } +void co_ifaces_shutdown(void) { + co_obj_free(ifaces); +} + int co_iface_remove(char *iface_name) { co_obj_t *iface = NULL; CHECK((iface = co_list_parse(ifaces, _co_iface_match_i, iface_name)) != NULL, "Failed to delete interface %s!", iface_name); diff --git a/src/iface.h b/src/iface.h index de6008c..5f33840 100644 --- a/src/iface.h +++ b/src/iface.h @@ -75,6 +75,11 @@ struct co_iface_t { */ int co_ifaces_create(void); +/** + * @brief removes all interfaces + */ +void co_ifaces_shutdown(void); + /** * @brief removes an interface from the list of available interfaces * @param iface_name name of interface to be removed diff --git a/src/profile.c b/src/profile.c index b610500..ae01ee2 100644 --- a/src/profile.c +++ b/src/profile.c @@ -113,6 +113,8 @@ co_profiles_shutdown(void) { if(_profiles != NULL) co_obj_free(_profiles); if(_profile_global != NULL) co_obj_free(_profile_global); + if(_schemas != NULL) co_obj_free(_schemas); + if(_schemas_global != NULL) co_obj_free(_schemas_global); return; } @@ -233,8 +235,11 @@ static char *_co_json_token_stringify(char *json, const jsmntok_t *token) } static int _co_profile_import_files_i(const char *path, const char *filename) { + int ret = 0; char path_tmp[PATH_MAX] = {}; FILE *config_file = NULL; + jsmntok_t *tokens = NULL; + char *buffer = NULL; DEBUG("Importing file %s at path %s", filename, path); @@ -246,12 +251,13 @@ static int _co_profile_import_files_i(const char *path, const char *filename) { fseek(config_file, 0, SEEK_END); long fsize = ftell(config_file); rewind(config_file); - char *buffer = h_calloc(1, fsize + 1); + buffer = h_calloc(1, fsize + 1); CHECK(fread(buffer, fsize, 1, config_file) != 0, "Failed to read from file."); fclose(config_file); + config_file = NULL; buffer[fsize] = '\0'; - jsmntok_t *tokens = _co_json_string_tokenize(buffer); + tokens = _co_json_string_tokenize(buffer); typedef enum { START, KEY, VALUE, STOP } parse_state; parse_state state = START; @@ -329,12 +335,14 @@ static int _co_profile_import_files_i(const char *path, const char *filename) { co_list_append(_profiles, new_profile); - return 1; + ret = 1; error: if(config_file != NULL) fclose(config_file); - if(new_profile != NULL) co_obj_free(new_profile); - return 0; + if(!ret && new_profile != NULL) co_obj_free(new_profile); + if (tokens) h_free(tokens); + if (buffer) h_free(buffer); + return ret; } int co_profile_import_files(const char *path) { @@ -348,7 +356,9 @@ int co_profile_import_files(const char *path) { } int co_profile_import_global(const char *path) { + int ret = 0; FILE *config_file = NULL; + jsmntok_t *tokens = NULL; if(_profile_global == NULL) { @@ -366,9 +376,10 @@ int co_profile_import_global(const char *path) { char *buffer = h_calloc(1, fsize + 1); CHECK(fread(buffer, fsize, 1, config_file) != 0, "Failed to read from file."); fclose(config_file); + config_file = NULL; buffer[fsize] = '\0'; - jsmntok_t *tokens = _co_json_string_tokenize(buffer); + tokens = _co_json_string_tokenize(buffer); typedef enum { START, KEY, VALUE, STOP } parse_state; parse_state state = START; @@ -442,11 +453,13 @@ int co_profile_import_global(const char *path) { } } - return 1; + ret = 1; error: if(config_file != NULL) fclose(config_file); - return 0; + if (buffer) h_free(buffer); + if (tokens) h_free(tokens); + return ret; } co_obj_t * diff --git a/src/profile.h b/src/profile.h index c0b9f29..361c0c9 100644 --- a/src/profile.h +++ b/src/profile.h @@ -39,7 +39,11 @@ #define SCHEMA(N) static int schema_##N(co_obj_t *self, co_obj_t **output, co_obj_t *params) -#define SCHEMA_ADD(K, V) co_tree_insert(self, K, sizeof(K), co_str8_create(V, sizeof(V), 0)) +#define SCHEMA_ADD(K, V) ({ \ + co_obj_t *val = co_str8_create(V, sizeof(V), 0); \ + if (!co_tree_insert(self, K, sizeof(K), val)) \ + co_obj_free(val); \ + }) #define SCHEMA_REGISTER(N) co_schema_register(schema_##N) From 64535953bb422cb048b6e33ebdbe07022ab9b2b1 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 15 Apr 2014 15:01:17 -0400 Subject: [PATCH 3/3] serval-dna global context --- plugins/serval-dna/serval-dna.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/plugins/serval-dna/serval-dna.c b/plugins/serval-dna/serval-dna.c index d148519..3f3e07b 100644 --- a/plugins/serval-dna/serval-dna.c +++ b/plugins/serval-dna/serval-dna.c @@ -81,6 +81,7 @@ static co_obj_t *co_alarm_create(struct sched_ent *alarm) { extern keyring_file *keyring; // Serval global extern char *serval_path; co_socket_t co_socket_proto = {}; +svl_crypto_ctx *global_ctx = NULL; static co_obj_t *sock_alarms = NULL; static co_obj_t *timer_alarms = NULL; @@ -395,7 +396,7 @@ static int serval_load_config(void) { int co_plugin_init(co_obj_t *self, co_obj_t **output, co_obj_t *params) { char *enabled = NULL; - svl_crypto_ctx *ctx = NULL; + global_ctx = NULL; co_profile_get_str(co_profile_global(),&enabled,"servald",sizeof("servald")); if (strcmp(enabled,"disabled") == 0) return 1; @@ -404,9 +405,9 @@ int co_plugin_init(co_obj_t *self, co_obj_t **output, co_obj_t *params) { srandomdev(); CHECK(serval_load_config(),"Failed to load Serval config parameters"); - ctx = svl_crypto_ctx_new(); - CHECK(serval_open_keyring(ctx),"Failed to open keyring"); - keyring = ctx->keyring_file; + global_ctx = svl_crypto_ctx_new(); + CHECK(serval_open_keyring(global_ctx),"Failed to open keyring"); + keyring = global_ctx->keyring_file; if (!serval_registered) { // CHECK(serval_register(),"Failed to register Serval commands"); @@ -436,8 +437,8 @@ int co_plugin_init(co_obj_t *self, co_obj_t **output, co_obj_t *params) { return 1; error: - if (ctx) - svl_crypto_ctx_free(ctx); + if (global_ctx) + svl_crypto_ctx_free(global_ctx); return 0; } @@ -470,6 +471,8 @@ int co_plugin_shutdown(co_obj_t *self, co_obj_t **output, co_obj_t *params) { daemon_started = false; + svl_crypto_ctx_free(global_ctx); + return 1; }