Skip to content

Commit

Permalink
gluterd: fix memory leak in glusterd (#4384)
Browse files Browse the repository at this point in the history 
* glusterd: fix memory leaks detected by asan

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 <[email protected]>

* 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 <[email protected]>

---------

Signed-off-by: chenjinhao <[email protected]>
Co-authored-by: chenjinhao <[email protected]>
  • Loading branch information
@chen1195585098
chen1195585098 and chenjinhao authored Jul 3, 2024
1 parent 00bb343 commit 599c608
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 3 deletions.
18 changes: 17 additions & 1 deletion xlators/mgmt/glusterd/src/glusterd-store.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2338,6 +2338,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);
}
Expand Down Expand Up @@ -2870,6 +2874,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");
Expand Down Expand Up @@ -3015,6 +3023,10 @@ glusterd_store_retrieve_node_state(glusterd_volinfo_t *volinfo)

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);
Expand Down Expand Up @@ -3257,7 +3269,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,
Expand Down Expand Up @@ -4607,6 +4619,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;
}
Expand Down
5 changes: 4 additions & 1 deletion xlators/mgmt/glusterd/src/glusterd-utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion xlators/storage/posix/src/posix-helpers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -3325,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"
Expand Down

0 comments on commit 599c608

Please sign in to comment.