From 152907d438bf8bb571e43ec278ec7b01da2431a6 Mon Sep 17 00:00:00 2001
From: Kotresh HR <khiremat@redhat.com>
Date: Wed, 5 Dec 2018 17:27:07 +0530
Subject: cluster/afr: Fix mem leak reported by ASAN

Traceback:

Direct leak of 765 byte(s) in 9 object(s) allocated from:
    #0 0x7ffb9cad2c48 in malloc (/lib64/libasan.so.5+0xeec48)
    #1 0x7ffb9c5f8949 in __gf_malloc ./libglusterfs/src/mem-pool.c:136
    #2 0x7ffb9c5f91bb in gf_vasprintf ./libglusterfs/src/mem-pool.c:236
    #3 0x7ffb9c5f938a in gf_asprintf ./libglusterfs/src/mem-pool.c:256
    #4 0x7ffb826714ab in afr_get_heal_info ./xlators/cluster/afr/src/afr-common.c:6204
    #5 0x7ffb825765e5 in afr_handle_heal_xattrs ./xlators/cluster/afr/src/afr-inode-read.c:1481
    #6 0x7ffb825765e5 in afr_getxattr ./xlators/cluster/afr/src/afr-inode-read.c:1571
    #7 0x7ffb9c635af7 in syncop_getxattr ./libglusterfs/src/syncop.c:1680
    #8 0x406c78 in glfsh_process_entries ./heal/src/glfs-heal.c:810
    #9 0x408555 in glfsh_crawl_directory ./heal/src/glfs-heal.c:898
    #10 0x408cc0 in glfsh_print_pending_heals_type ./heal/src/glfs-heal.c:970
    #11 0x408fc5 in glfsh_print_pending_heals ./heal/src/glfs-heal.c:1012
    #12 0x409546 in glfsh_gather_heal_info ./heal/src/glfs-heal.c:1154
    #13 0x403e96 in main ./heal/src/glfs-heal.c:1745
    #14 0x7ffb99bc411a in __libc_start_main ../csu/libc-start.c:308

The dictionary is referenced by caller to print the status.
So set it as dynstr, the last unref of dictionary will free it.

updates: bz#1633930
Change-Id: Ib5a7cb891e6f7d90560859aaf6239e52ff5477d0
Signed-off-by: Kotresh HR <khiremat@redhat.com>
---
 xlators/cluster/afr/src/afr-common.c | 43 ++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

(limited to 'xlators')

diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 52c9f2cfd58..9137c595d37 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -6141,13 +6141,22 @@ afr_set_heal_info(char *status)
         goto out;
     }
 
-    ret = dict_set_str(dict, "heal-info", status);
+    ret = dict_set_dynstr(dict, "heal-info", status);
     if (ret)
         gf_msg("", GF_LOG_WARNING, -ret, AFR_MSG_DICT_SET_FAILED,
                "Failed to set heal-info key to "
                "%s",
                status);
 out:
+    /* Any error other than EINVAL, dict_set_dynstr frees status */
+    if (ret == -ENOMEM || ret == -EINVAL) {
+        GF_FREE(status);
+    }
+
+    if (ret && dict) {
+        dict_unref(dict);
+        dict = NULL;
+    }
     return dict;
 }
 
@@ -6160,7 +6169,7 @@ afr_get_heal_info(call_frame_t *frame, xlator_t *this, loc_t *loc)
     unsigned char pending = 0;
     dict_t *dict = NULL;
     int ret = -1;
-    int op_errno = 0;
+    int op_errno = ENOMEM;
     inode_t *inode = NULL;
     char *substr = NULL;
     char *status = NULL;
@@ -6170,7 +6179,6 @@ afr_get_heal_info(call_frame_t *frame, xlator_t *this, loc_t *loc)
                                       &metadata_selfheal, &pending);
 
     if (ret == -ENOMEM) {
-        op_errno = -ret;
         ret = -1;
         goto out;
     }
@@ -6186,23 +6194,44 @@ afr_get_heal_info(call_frame_t *frame, xlator_t *this, loc_t *loc)
         if (ret < 0)
             goto out;
         dict = afr_set_heal_info(status);
+        if (!dict) {
+            ret = -1;
+            goto out;
+        }
     } else if (ret == -EAGAIN) {
         ret = gf_asprintf(&status, "possibly-healing%s", substr ? substr : "");
         if (ret < 0)
             goto out;
         dict = afr_set_heal_info(status);
+        if (!dict) {
+            ret = -1;
+            goto out;
+        }
     } else if (ret >= 0) {
         /* value of ret = source index
          * so ret >= 0 and at least one of the 3 booleans set to
          * true means a source is identified; heal is required.
          */
         if (!data_selfheal && !entry_selfheal && !metadata_selfheal) {
-            dict = afr_set_heal_info("no-heal");
+            status = gf_strdup("no-heal");
+            if (!status) {
+                ret = -1;
+                goto out;
+            }
+            dict = afr_set_heal_info(status);
+            if (!dict) {
+                ret = -1;
+                goto out;
+            }
         } else {
             ret = gf_asprintf(&status, "heal%s", substr ? substr : "");
             if (ret < 0)
                 goto out;
             dict = afr_set_heal_info(status);
+            if (!dict) {
+                ret = -1;
+                goto out;
+            }
         }
     } else if (ret < 0) {
         /* Apart from above checked -ve ret values, there are
@@ -6217,9 +6246,15 @@ afr_get_heal_info(call_frame_t *frame, xlator_t *this, loc_t *loc)
             if (ret < 0)
                 goto out;
             dict = afr_set_heal_info(status);
+            if (!dict) {
+                ret = -1;
+                goto out;
+            }
         }
     }
+
     ret = 0;
+    op_errno = 0;
 
 out:
     AFR_STACK_UNWIND(getxattr, frame, ret, op_errno, dict, NULL);
-- 
cgit