diff options
| author | Kotresh HR <khiremat@redhat.com> | 2018-12-05 17:27:07 +0530 | 
|---|---|---|
| committer | Pranith Kumar Karampuri <pkarampu@redhat.com> | 2018-12-17 04:46:08 +0000 | 
| commit | 152907d438bf8bb571e43ec278ec7b01da2431a6 (patch) | |
| tree | 1f1d46b3703b8d01ca4ece0b60176c406f73856a | |
| parent | b40bb5394dbe5b391d8218d71d672d7e1820e5d3 (diff) | |
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>
| -rw-r--r-- | xlators/cluster/afr/src/afr-common.c | 43 | 
1 files changed, 39 insertions, 4 deletions
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);  | 
