From 9827700256fae4b2aec3a8a153af53981daae1cd Mon Sep 17 00:00:00 2001 From: cenhuilin Date: Mon, 22 Jul 2024 11:25:45 +0800 Subject: [PATCH] glusterd: fix memory leak in glusterd (cherry picked from commit 09a0a2ed5b8cb343206d27caa22e23901e6b6511) --- ...-memory-leaks-due-to-lack-of-GF_FREE.patch | 110 +++++++++++++++++ ...rd-fix-memory-leaks-detected-by-asan.patch | 112 ++++++++++++++++++ glusterfs.spec | 7 +- 3 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 0005-glusterd-fix-memory-leaks-due-to-lack-of-GF_FREE.patch create mode 100644 0006-glusterd-fix-memory-leaks-detected-by-asan.patch diff --git a/0005-glusterd-fix-memory-leaks-due-to-lack-of-GF_FREE.patch b/0005-glusterd-fix-memory-leaks-due-to-lack-of-GF_FREE.patch new file mode 100644 index 0000000..384fe5a --- /dev/null +++ b/0005-glusterd-fix-memory-leaks-due-to-lack-of-GF_FREE.patch @@ -0,0 +1,110 @@ +From a910f59d534e19bd6fca5aa0e47b40afbbfc7f56 Mon Sep 17 00:00:00 2001 +From: chenjinhao +Date: Mon, 22 Jul 2024 11:10:28 +0800 +Subject: [PATCH] glusterd: fix memory leaks due to lack of GF_FREE + +In glusterd-store.c, there are while loops like: + +gf_store_iter_get_next(iter, &key, &value, &op_errno); +while (!ret) { + if (xx_condition) { + do_something(); + goto out; + } + GF_FREE(key); + GF_FREE(value); + key = NULL; + value = NULL; + + ret = gf_store_iter_get_next(iter, &key, &value, &op_errno); + +} +It's ok under normal case, howerver, once the condition does not meet +and the procedure goto 'out', there will be memory leak. + +Hence, it is necessary to put a check at 'out'. + +Similar leaks will be triggered in glusterd_store_retrieve_peers. +If no peerinfo is found, the procedure will goto the next loop. +It means memory previously allocated for key & value will be +leaked once gf_store_iter_get_next is called again in the next loop. + +Signed-off-by: chenjinhao +--- + xlators/mgmt/glusterd/src/glusterd-store.c | 16 ++++++++++++++++ + xlators/storage/posix/src/posix-helpers.c | 4 ++++ + 2 files changed, 20 insertions(+) + +diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c +index 42d82d8..8594fc9 100644 +--- a/xlators/mgmt/glusterd/src/glusterd-store.c ++++ b/xlators/mgmt/glusterd/src/glusterd-store.c +@@ -2364,6 +2364,10 @@ glusterd_store_retrieve_snapd(glusterd_volinfo_t *volinfo) + SLEN(GLUSTERD_STORE_KEY_SNAPD_PORT))) { + volinfo->snapd.port = atoi(value); + } ++ GF_FREE(key); ++ GF_FREE(value); ++ key = NULL; ++ value = NULL; + + ret = gf_store_iter_get_next(iter, &key, &value, &op_errno); + } +@@ -2896,6 +2900,10 @@ glusterd_store_retrieve_bricks(glusterd_volinfo_t *volinfo) + ret = 0; + + out: ++ if (key) ++ GF_FREE(key); ++ if (value) ++ GF_FREE(value); + 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"); +@@ -3041,6 +3049,10 @@ out: + + if (dup_value) + GF_FREE(dup_value); ++ if (key) ++ GF_FREE(key); ++ if (value) ++ GF_FREE(value); + if (ret) { + if (volinfo->rebal.dict) + dict_unref(volinfo->rebal.dict); +@@ -4633,6 +4645,10 @@ glusterd_store_retrieve_peers(xlator_t *this) + peerinfo = glusterd_peerinfo_new(GD_FRIEND_STATE_DEFAULT, NULL, NULL, + 0); + if (peerinfo == NULL) { ++ GF_FREE(key); ++ GF_FREE(value); ++ key = NULL; ++ value = NULL; + ret = -1; + goto next; + } +diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c +index 08bb4ac..9fae102 100644 +--- a/xlators/storage/posix/src/posix-helpers.c ++++ b/xlators/storage/posix/src/posix-helpers.c +@@ -3288,6 +3288,8 @@ posix_cs_set_state(xlator_t *this, dict_t **rsp, gf_cs_obj_state state, + xattrsize = sys_fgetxattr(*fd, GF_CS_OBJECT_REMOTE, value, + xattrsize + 1); + if (xattrsize == -1) { ++ if (value) ++ GF_FREE(value); + gf_msg(this->name, GF_LOG_ERROR, 0, errno, + " getxattr failed for key %s", GF_CS_OBJECT_REMOTE); + goto out; +@@ -3311,6 +3313,8 @@ posix_cs_set_state(xlator_t *this, dict_t **rsp, gf_cs_obj_state state, + xattrsize = sys_lgetxattr(path, GF_CS_OBJECT_REMOTE, value, + xattrsize + 1); + if (xattrsize == -1) { ++ if (value) ++ GF_FREE(value); + gf_msg(this->name, GF_LOG_ERROR, 0, errno, + " getxattr failed for key %s", GF_CS_OBJECT_REMOTE); + goto out; +-- +2.33.0 + diff --git a/0006-glusterd-fix-memory-leaks-detected-by-asan.patch b/0006-glusterd-fix-memory-leaks-detected-by-asan.patch new file mode 100644 index 0000000..756283c --- /dev/null +++ b/0006-glusterd-fix-memory-leaks-detected-by-asan.patch @@ -0,0 +1,112 @@ +From b9a37a2366bbfddd47da81bb86df97a9e6431d85 Mon Sep 17 00:00:00 2001 +From: chenjinhao +Date: Mon, 22 Jul 2024 11:15:58 +0800 +Subject: [PATCH] glusterd: fix memory leaks detected by asan +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Memory leaks detected by setting --enable-asan, compile, install +and run gluster cmds, such as gluster v create/start/stop, etc. + +Direct leak of 11430 byte(s) in 150 object(s) allocated from: + #0 0x7f59844efbb8 in __interceptor_malloc (/lib64/libasan.so.5+0xefbb8) + #1 0x7f5983aeb96d in __gf_malloc (/lib64/libglusterfs.so.0+0xeb96d) + #2 0x7f59775b569b in glusterd_store_update_volinfo ../../../../libglusterfs/src/glusterfs/mem-pool.h:170 + #3 0x7f59775be3b5 in glusterd_store_retrieve_volume glusterd-store.c:3334 + #4 0x7f59775bf076 in glusterd_store_retrieve_volumes glusterd-store.c:3571 + #5 0x7f59775bfc9e in glusterd_restore glusterd-store.c:4913 + #6 0x7f59774ca5a1 (/usr/lib64/glusterfs/8.2/xlator/mgmt/glusterd.so+0xca5a1) + #7 0x7f5983a7cb6b in __xlator_init xlator.c:594 + #8 0x7f5983b0c5d0 in glusterfs_graph_init graph.c:422 + #9 0x7f5983b0d422 in glusterfs_graph_activate (/lib64/libglusterfs.so.0+0x10d422) + #10 0x5605f2e1eff5 in glusterfs_process_volfp glusterfsd.c:2506 + #11 0x5605f2e1f238 in glusterfs_volumes_init glusterfsd.c:2577 + #12 0x5605f2e15d8d in main (/usr/sbin/glusterfsd+0x15d8d) + #13 0x7f598103acf2 in __libc_start_main (/lib64/libc.so.6+0x3acf2) + #14 0x5605f2e162cd in _start (/usr/sbin/glusterfsd+0x162cd) +In glusterd_store_update_volinfo, memory will be leaked when the dynamic memory is put +into a dict by calling dict_set_str: +ret = dict_set_str(volinfo->dict, key, gf_strdup(value)); + +Direct leak of 3351 byte(s) in 30 object(s) allocated from: + #0 0x7f59844efbb8 in __interceptor_malloc (/lib64/libasan.so.5+0xefbb8) + #1 0x7f5983aeb96d in __gf_malloc (/lib64/libglusterfs.so.0+0xeb96d) + #2 0x7f59775541e7 in glusterd_is_path_in_use ../../../../libglusterfs/src/glusterfs/mem-pool.h:170 + #3 0x7f59775541e7 in glusterd_check_and_set_brick_xattr glusterd-utils.c:8203 + #4 0x7f5977554d7c in glusterd_validate_and_create_brickpath glusterd-utils.c:1549 + #5 0x7f5977645fcb in glusterd_op_stage_create_volume glusterd-volume-ops.c:1260 + #6 0x7f5977519025 in glusterd_op_stage_validate glusterd-op-sm.c:5787 + #7 0x7f5977672555 in gd_stage_op_phase glusterd-syncop.c:1297 + #8 0x7f5977676db0 in gd_sync_task_begin glusterd-syncop.c:1951 + #9 0x7f59776775dc in glusterd_op_begin_synctask glusterd-syncop.c:2016 + #10 0x7f5977642bd6 in __glusterd_handle_create_volume glusterd-volume-ops.c:506 + #11 0x7f59774e27b1 in glusterd_big_locked_handler glusterd-handler.c:83 + #12 0x7f5983b14cac in synctask_wrap syncop.c:353 + #13 0x7f59810240af (/lib64/libc.so.6+0x240af) +During volume creation, glusterd_is_path_in_use will be called to check brick path reuse. +Under normal circumstances, there is no problem， however, when a `force` cmd is given during +volume creation and the futher sys_lsetxattr failed, the memory area previously pointed by +*op_errstr will be leakd, cause: +out: + if (strlen(msg)) + *op_errstr = gf_strdup(msg); + +Similar leak also exists in posix_cs_set_state: +value = GF_CALLOC(1, xattrsize + 1, gf_posix_mt_char); +... +dict_set_str_sizen(*rsp, GF_CS_OBJECT_REMOTE, value); + +Signed-off-by: chenjinhao +--- + xlators/mgmt/glusterd/src/glusterd-store.c | 2 +- + xlators/mgmt/glusterd/src/glusterd-utils.c | 5 ++++- + xlators/storage/posix/src/posix-helpers.c | 2 +- + 3 files changed, 6 insertions(+), 3 deletions(-) + +diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c +index 8594fc9..ce3d7a4 100644 +--- a/xlators/mgmt/glusterd/src/glusterd-store.c ++++ b/xlators/mgmt/glusterd/src/glusterd-store.c +@@ -3295,7 +3295,7 @@ glusterd_store_update_volinfo(glusterd_volinfo_t *volinfo) + */ + if (!strcmp(key, "features.limit-usage")) + break; +- ret = dict_set_str(volinfo->dict, key, gf_strdup(value)); ++ ret = dict_set_dynstr(volinfo->dict, key, gf_strdup(value)); + if (ret) { + gf_msg(this->name, GF_LOG_ERROR, 0, + GD_MSG_DICT_SET_FAILED, +diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c +index 72352ce..e255e99 100644 +--- a/xlators/mgmt/glusterd/src/glusterd-utils.c ++++ b/xlators/mgmt/glusterd/src/glusterd-utils.c +@@ -7754,8 +7754,11 @@ glusterd_check_and_set_brick_xattr(char *host, char *path, uuid_t uuid, + + ret = 0; + out: +- if (strlen(msg)) ++ if (strlen(msg)) { ++ if (*op_errstr) ++ GF_FREE(*op_errstr); + *op_errstr = gf_strdup(msg); ++ } + + return ret; + } +diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c +index 9fae102..df3ed73 100644 +--- a/xlators/storage/posix/src/posix-helpers.c ++++ b/xlators/storage/posix/src/posix-helpers.c +@@ -3329,7 +3329,7 @@ posix_cs_set_state(xlator_t *this, dict_t **rsp, gf_cs_obj_state state, + } + + if (ret == 0) { +- ret = dict_set_str_sizen(*rsp, GF_CS_OBJECT_REMOTE, value); ++ ret = dict_set_dynstr_sizen(*rsp, GF_CS_OBJECT_REMOTE, value); + if (ret) { + gf_msg(this->name, GF_LOG_ERROR, 0, 0, + "failed to set" +-- +2.33.0 + diff --git a/glusterfs.spec b/glusterfs.spec index 1da7715..440950b 100644 --- a/glusterfs.spec +++ b/glusterfs.spec @@ -224,7 +224,7 @@ Summary: Distributed File System Name: glusterfs Version: 11.1 -Release: 4 +Release: 5 License: GPLv3 or GPLv2+ or LGPLv3+ URL: http://docs.gluster.org/ %if ( 0%{_for_fedora_koji_builds} ) @@ -241,6 +241,8 @@ Patch1: 0001-fuse-Resolve-asan-bug-in-during-receive-event-notifi.patch Patch2: 0002-fix-Hostname-validation.patch Patch3: 0003-fix-mount.glusterfs-Remove-from-grep-command.patch Patch4: 0004-prevent-gnfs-IO-Errors-on-smaller-files.patch +Patch5: 0005-glusterd-fix-memory-leaks-due-to-lack-of-GF_FREE.patch +Patch6: 0006-glusterd-fix-memory-leaks-detected-by-asan.patch BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) BuildRequires: rpcgen gperftools-devel libunwind-devel @@ -1503,6 +1505,9 @@ exit 0 %{_mandir}/man8/*gluster*.8* %changelog +* Mon Jul 22 2024 cenhuilin - 11.1-5 +- glusterd: fix memory leak in glusterd + * Wed Jun 19 2024 zhangyaqi - 11.1-4 - fix prevent gnfs IO Errors on smaller files -- Gitee