gluterd: fix memory leak in glusterd (#4384)

* 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]>

Author: chen1195585098

xlators/mgmt/glusterd/src/glusterd-store.c

@@ -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);
     }
@@ -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");
@@ -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);
@@ -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,
@@ -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;
         }

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;
 }

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;
@@ -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"