From 59c05230c0df58765e30553c66bbcc0c9965d362 Mon Sep 17 00:00:00 2001 From: nik-redhat Date: Tue, 11 Aug 2020 23:12:26 +0530 Subject: [PATCH 597/610] glusterd: memory deallocated twice Issue: If the the pointer tmptier is destroyed in the function code it still it checks for the same in the out label. And tries to destroy the same pointer again. Fix: So, instead of passing the ptr by value, if we pass it by reference then, on making the ptr in the function the value will persist, in the calling function and next time when the gf_store_iter_destory() is called it won't try to free the ptr again. CID: 1430122 >Updates: #1060 >Change-Id: I019cea8e301c7cc87be792c03b58722fc96f04ef >Signed-off-by: nik-redhat Upstream link: https://review.gluster.org/c/glusterfs/+/24855 BUG: 1997447 Change-Id: Ib403efd08d47a69d25f291ae61c9cbfcaaa05da8 Signed-off-by: nik-redhat Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/280076 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- libglusterfs/src/glusterfs/store.h | 2 +- libglusterfs/src/store.c | 12 +++++++----- xlators/mgmt/glusterd/src/glusterd-store.c | 16 ++++++++-------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/libglusterfs/src/glusterfs/store.h b/libglusterfs/src/glusterfs/store.h index 68a20ad..76af2df 100644 --- a/libglusterfs/src/glusterfs/store.h +++ b/libglusterfs/src/glusterfs/store.h @@ -93,7 +93,7 @@ int32_t gf_store_iter_get_matching(gf_store_iter_t *iter, char *key, char **value); int32_t -gf_store_iter_destroy(gf_store_iter_t *iter); +gf_store_iter_destroy(gf_store_iter_t **iter); char * gf_store_strerror(gf_store_op_errno_t op_errno); diff --git a/libglusterfs/src/store.c b/libglusterfs/src/store.c index 3af627a..e4931bf 100644 --- a/libglusterfs/src/store.c +++ b/libglusterfs/src/store.c @@ -606,23 +606,25 @@ out: } int32_t -gf_store_iter_destroy(gf_store_iter_t *iter) +gf_store_iter_destroy(gf_store_iter_t **iter) { int32_t ret = -1; - if (!iter) + if (!(*iter)) return 0; /* gf_store_iter_new will not return a valid iter object with iter->file * being NULL*/ - ret = fclose(iter->file); + ret = fclose((*iter)->file); if (ret) gf_msg("", GF_LOG_ERROR, errno, LG_MSG_FILE_OP_FAILED, "Unable" " to close file: %s, ret: %d", - iter->filepath, ret); + (*iter)->filepath, ret); + + GF_FREE(*iter); + *iter = NULL; - GF_FREE(iter); return ret; } diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c index a8651d8..e027575 100644 --- a/xlators/mgmt/glusterd/src/glusterd-store.c +++ b/xlators/mgmt/glusterd/src/glusterd-store.c @@ -2576,7 +2576,7 @@ glusterd_store_retrieve_snapd(glusterd_volinfo_t *volinfo) ret = 0; out: - if (gf_store_iter_destroy(iter)) { + if (gf_store_iter_destroy(&iter)) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_STORE_ITER_DESTROY_FAIL, "Failed to destroy store iter"); ret = -1; @@ -2895,13 +2895,13 @@ glusterd_store_retrieve_bricks(glusterd_volinfo_t *volinfo) ret = 0; out: - if (gf_store_iter_destroy(tmpiter)) { + if (gf_store_iter_destroy(&tmpiter)) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_STORE_ITER_DESTROY_FAIL, "Failed to destroy store iter"); ret = -1; } - if (gf_store_iter_destroy(iter)) { + if (gf_store_iter_destroy(&iter)) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_STORE_ITER_DESTROY_FAIL, "Failed to destroy store iter"); ret = -1; @@ -3067,7 +3067,7 @@ glusterd_store_retrieve_node_state(glusterd_volinfo_t *volinfo) ret = 0; out: - if (gf_store_iter_destroy(iter)) { + if (gf_store_iter_destroy(&iter)) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_STORE_ITER_DESTROY_FAIL, "Failed to destroy store iter"); ret = -1; @@ -3379,7 +3379,7 @@ glusterd_store_update_volinfo(glusterd_volinfo_t *volinfo) ret = 0; out: - if (gf_store_iter_destroy(iter)) { + if (gf_store_iter_destroy(&iter)) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_STORE_ITER_DESTROY_FAIL, "Failed to destroy store iter"); ret = -1; @@ -3574,7 +3574,7 @@ glusterd_store_retrieve_options(xlator_t *this) goto out; ret = 0; out: - (void)gf_store_iter_destroy(iter); + (void)gf_store_iter_destroy(&iter); gf_store_handle_destroy(shandle); return ret; } @@ -4026,7 +4026,7 @@ glusterd_store_update_snap(glusterd_snap_t *snap) ret = 0; out: - if (gf_store_iter_destroy(iter)) { + if (gf_store_iter_destroy(&iter)) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_STORE_ITER_DESTROY_FAIL, "Failed to destroy store iter"); ret = -1; @@ -4774,7 +4774,7 @@ glusterd_store_retrieve_peers(xlator_t *this) is_ok = _gf_true; next: - (void)gf_store_iter_destroy(iter); + (void)gf_store_iter_destroy(&iter); if (!is_ok) { gf_log(this->name, GF_LOG_WARNING, -- 1.8.3.1