From eacc48b96b818ab2d15ed9d3a9a21818feed2826 Mon Sep 17 00:00:00 2001 From: Sunny Kumar Date: Thu, 24 Jan 2019 12:14:48 +0530 Subject: cli : fix mem leak during cli_cmd_volume_gsync_set_cbk This patch fixes mem-leak due to excessive use of gf_asprintf to form dynamic growing string. Problem: each call to asprintf/vsprintf for extending existing string causes a memory leak, because the blob at the original address of existing string is not freed and a new location is generated by asprintf. Tracebacks: #2 0x7fdf191b8b3b in gf_vasprintf ../libglusterfs/src/mem-pool.c:236 #3 0x7fdf191b8d0a in gf_asprintf ../libglusterfs/src/mem-pool.c:256 #4 0x420cd3 in cli_cmd_volume_gsync_set_cbk ../cli/src/cli-cmd-volume.c:2576 SUMMARY: AddressSanitizer: 255 byte(s) leaked in 3 allocation(s). .... SUMMARY: AddressSanitizer: 431 byte(s) leaked in 4 allocation(s). .... SUMMARY: AddressSanitizer: 449 byte(s) leaked in 4 allocation(s). .... SUMMARY: AddressSanitizer: 397 byte(s) leaked in 4 allocation(s). .... SUMMARY: AddressSanitizer: 160 byte(s) leaked in 2 allocation(s). .... updates: bz#1633930 Change-Id: I7e8902f0ed23e640dc17e3dcbdab7ae0579d2dc6 Signed-off-by: Sunny Kumar --- cli/src/cli-cmd-volume.c | 70 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c index 3432dbebd8..585865f562 100644 --- a/cli/src/cli-cmd-volume.c +++ b/cli/src/cli-cmd-volume.c @@ -34,6 +34,9 @@ extern struct rpc_clnt *global_quotad_rpc; extern rpc_clnt_prog_t *cli_rpc_prog; extern rpc_clnt_prog_t cli_quotad_clnt; +static int +gf_asprintf_append(char **string_ptr, const char *format, ...); + int cli_cmd_volume_help_cbk(struct cli_state *state, struct cli_cmd_word *in_word, const char **words, int wordcount); @@ -2544,13 +2547,15 @@ out: if (ret1) tmp = ""; - gf_asprintf(&events_str, "%soption=%s;", events_str, tmp); + gf_asprintf_append(&events_str, "%soption=%s;", events_str, + tmp); ret1 = dict_get_str(options, "op_value", &tmp); if (ret1) tmp = ""; - gf_asprintf(&events_str, "%svalue=%s;", events_str, tmp); + gf_asprintf_append(&events_str, "%svalue=%s;", events_str, + tmp); } else if (strcmp(tmp, "del") == 0) { event_type = EVENT_GEOREP_CONFIG_RESET; @@ -2558,7 +2563,8 @@ out: if (ret1) tmp = ""; - gf_asprintf(&events_str, "%soption=%s;", events_str, tmp); + gf_asprintf_append(&events_str, "%soption=%s;", events_str, + tmp); } break; default: @@ -2568,42 +2574,49 @@ out: if (event_type > -1) { /* Capture all optional arguments used */ ret1 = dict_get_int32(options, "force", &tmpi); - if (ret1 == 0) - gf_asprintf(&events_str, "%sforce=%d;", events_str, tmpi); - + if (ret1 == 0) { + gf_asprintf_append(&events_str, "%sforce=%d;", events_str, + tmpi); + } ret1 = dict_get_int32(options, "push_pem", &tmpi); - if (ret1 == 0) - gf_asprintf(&events_str, "%spush_pem=%d;", events_str, tmpi); - + if (ret1 == 0) { + gf_asprintf_append(&events_str, "%spush_pem=%d;", events_str, + tmpi); + } ret1 = dict_get_int32(options, "no_verify", &tmpi); - if (ret1 == 0) - gf_asprintf(&events_str, "%sno_verify=%d;", events_str, tmpi); + if (ret1 == 0) { + gf_asprintf_append(&events_str, "%sno_verify=%d;", events_str, + tmpi); + } ret1 = dict_get_int32(options, "ssh_port", &tmpi); - if (ret1 == 0) - gf_asprintf(&events_str, "%sssh_port=%d;", events_str, tmpi); + if (ret1 == 0) { + gf_asprintf_append(&events_str, "%sssh_port=%d;", events_str, + tmpi); + } ret1 = dict_get_int32(options, "reset-sync-time", &tmpi); - if (ret1 == 0) - gf_asprintf(&events_str, "%sreset_sync_time=%d;", events_str, - tmpi); - + if (ret1 == 0) { + gf_asprintf_append(&events_str, "%sreset_sync_time=%d;", + events_str, tmpi); + } /* Capture Master and Slave Info */ ret1 = dict_get_str(options, "master", &tmp); if (ret1) tmp = ""; - gf_asprintf(&events_str, "%smaster=%s;", events_str, tmp); + gf_asprintf_append(&events_str, "%smaster=%s;", events_str, tmp); ret1 = dict_get_str(options, "slave", &tmp); if (ret1) tmp = ""; - gf_asprintf(&events_str, "%sslave=%s", events_str, tmp); + gf_asprintf_append(&events_str, "%sslave=%s", events_str, tmp); gf_event(event_type, "%s", events_str); } /* Allocated by gf_strdup and gf_asprintf */ - GF_FREE(events_str); + if (events_str) + GF_FREE(events_str); } #endif @@ -3617,3 +3630,20 @@ cli_cmd_volume_register(struct cli_state *state) out: return ret; } + +static int +gf_asprintf_append(char **string_ptr, const char *format, ...) +{ + va_list arg; + int rv = 0; + char *tmp = *string_ptr; + + va_start(arg, format); + rv = gf_vasprintf(string_ptr, format, arg); + va_end(arg); + + if (tmp) + GF_FREE(tmp); + + return rv; +} -- cgit